third_party/WebKit/LayoutTests/http/tests/history/history-entry-requires-user-gesture.html - Issue 2780953002: When historyEntryRequiresUserGesture is enabled, exempt docs that have been committed for 5 seconds

Side by Side Diff: third_party/WebKit/LayoutTests/http/tests/history/history-entry-requires-user-gesture.html

Issue 2780953002: When historyEntryRequiresUserGesture is enabled, exempt docs that have been committed for 5 seconds (Closed)

Patch Set: Centralize checking the setting Created 3 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « no previous file | third_party/WebKit/LayoutTests/http/tests/history/history-entry-requires-user-gesture-expected.txt » ('j') | third_party/WebKit/Source/core/dom/Document.cpp » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <body onload="loaded();">
	ojan 2017/04/03 21:03:54 Should we also have a version of this test that ha Should we also have a version of this test that has a user gesture? Nate Chapin 2017/04/04 20:42:19 Added a 2nd step to this test, so there's one navi Show quoted text On 2017/04/03 21:03:54, ojan wrote: > Should we also have a version of this test that has a user gesture? Added a 2nd step to this test, so there's one navigation with a user gesture and one without.
	2 <script>

	3 if (window.testRunner) {

	4 testRunner.dumpAsText();

	5 testRunner.dumpBackForwardList();

	6 testRunner.waitUntilDone();

	7 internals.settings.setHistoryEntryRequiresUserGesture(true);
	ojan 2017/04/03 21:03:54 A downside of this solution is that we have to kee A downside of this solution is that we have to keep the setting around forever. Is this just a problem of taking the time to update the other tests? Is it hard to give them all a user gesture? I'd be fine with a TODO to update the tests when we remove this setting. Nate Chapin 2017/04/04 20:42:19 Time + churn. Giving them all a user gesture is a Show quoted text On 2017/04/03 21:03:54, ojan wrote: > A downside of this solution is that we have to keep the setting around forever. > > Is this just a problem of taking the time to update the other tests? Is it hard > to give them all a user gesture? > > I'd be fine with a TODO to update the tests when we remove this setting. Time + churn. Giving them all a user gesture is a fairly tedious but mechanical change. More importantly, quite a few of the tests that depend on the existing behavior are web-platform-tests, and I don't want to update those until this behavior is in the spec. I just don't think it makes sense to do it until this feature is proven, at which point we can spec it, update the tests, and remove the flag. Added TODO in Document::canCreateHistoryEntry(). Z_DONOTUSE 2017/04/05 17:39:13 Ah. That makes total sense. I wasn't thinking abou Show quoted text On 2017/04/04 at 20:42:19, Nate Chapin wrote: > On 2017/04/03 21:03:54, ojan wrote: > > A downside of this solution is that we have to keep the setting around forever. > > > > Is this just a problem of taking the time to update the other tests? Is it hard > > to give them all a user gesture? > > > > I'd be fine with a TODO to update the tests when we remove this setting. > > Time + churn. Giving them all a user gesture is a fairly tedious but mechanical change. More importantly, quite a few of the tests that depend on the existing behavior are web-platform-tests, and I don't want to update those until this behavior is in the spec. I just don't think it makes sense to do it until this feature is proven, at which point we can spec it, update the tests, and remove the flag. > > Added TODO in Document::canCreateHistoryEntry(). Ah. That makes total sense. I wasn't thinking about the web-platform-tests/spec side of this. A TODO SGTM.
	8 }

	9

	10 function loaded() {

	11 setTimeout("location = '../resources/notify-done.html'", 0);

	12 }

	13 </script>

	14 </body>

OLD	NEW