Bug 87668 - [EFL] Reset "WebKitTabToLinksPreferenceKey" in DumpRenderTreeChrome::resetDefaultsToConsistentValues
Summary: [EFL] Reset "WebKitTabToLinksPreferenceKey" in DumpRenderTreeChrome::resetDef...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mikhail Pozdnyakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-28 08:12 PDT by Mikhail Pozdnyakov
Modified: 2012-06-04 02:28 PDT (History)
5 users (show)

See Also:


Attachments
patch (1.94 KB, patch)
2012-05-28 08:17 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff
patch v2 (corrected changelog) (2.15 KB, patch)
2012-05-28 10:24 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff
patch v3 (removed EFL related part) (1.54 KB, patch)
2012-05-30 06:57 PDT, Mikhail Pozdnyakov
rniwa: review-
rniwa: commit-queue-
Details | Formatted Diff | Diff
patch v4 (reset "WebKitTabToLinksPreferenceKey" to false in EFL DRT) (1.54 KB, patch)
2012-06-04 00:57 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff
patch v4 (reset "WebKitTabToLinksPreferenceKey" to false in EFL DRT) (3.08 KB, patch)
2012-06-04 01:00 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Pozdnyakov 2012-05-28 08:12:13 PDT
LayoutTests/fast/html/tab-order.html has a link inside the test description which may be included in focus chain (if "WebKitTabToLinksPreferenceKey" setting is set) and lead to test failing. 

So think it would be nice to set to false "WebKitTabToLinksPreferenceKey" LayoutTestController setting before the test execution to avoid the mentioned problem and to be consistent with other tests (take for instance fast/events/tab-focus-anchor.html).
Comment 1 Mikhail Pozdnyakov 2012-05-28 08:17:47 PDT
Created attachment 144368 [details]
patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-05-28 09:29:41 PDT
Comment on attachment 144368 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144368&action=review

> LayoutTests/ChangeLog:3
> +        [DRT] LayoutTests/fast/html/tab-order.html should set to false "WebKitTabToLinksPreferenceKey"  LayoutTestController setting

The 'DRT' prefix here seems unnecessary. It'd be good to include your explanation in the ChangeLog.
Comment 3 Mikhail Pozdnyakov 2012-05-28 10:24:17 PDT
Created attachment 144382 [details]
patch v2 (corrected changelog)
Comment 4 Mikhail Pozdnyakov 2012-05-28 10:26:21 PDT
(In reply to comment #2)
> (From update of attachment 144368 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=144368&action=review
> 
> > LayoutTests/ChangeLog:3
> > +        [DRT] LayoutTests/fast/html/tab-order.html should set to false "WebKitTabToLinksPreferenceKey"  LayoutTestController setting
> 
> The 'DRT' prefix here seems unnecessary. It'd be good to include your explanation in the ChangeLog.

Done.
Comment 5 Gustavo Noronha (kov) 2012-05-29 09:40:19 PDT
Comment on attachment 144382 [details]
patch v2 (corrected changelog)

Why don't other ports need this? Should this setting be set to a better default when restoring state to sane defaults in EFL's DRT instead?
Comment 6 Mikhail Pozdnyakov 2012-05-30 00:23:49 PDT
(In reply to comment #5)
> (From update of attachment 144382 [details])
> Why don't other ports need this? Should this setting be set to a better default when restoring state to sane defaults in EFL's DRT instead?

The WebKit EFL problem can be solved if WebKitTabToLinksPreferenceKey is reset to false each time. However think it's sane to set it also inside the test since this setting is crucial for the test and its necessity is known beforehand (I mean this is not the same as uncleaned cache or forgotten user script -- those things must be handled in DRT)

BTW other tests set it (see example in the description of this bug).

I will remove the EFL specific part from the patch.
Comment 7 Mikhail Pozdnyakov 2012-05-30 06:57:07 PDT
Created attachment 144801 [details]
patch v3 (removed EFL related part)
Comment 8 Alexey Proskuryakov 2012-05-30 10:04:37 PDT
I agree with Gustavo - it's wrong to rely on a test to reset preferences to original state itself. Very few tests do that.
Comment 9 Ryosuke Niwa 2012-06-01 00:20:06 PDT
Comment on attachment 144801 [details]
patch v3 (removed EFL related part)

View in context: https://bugs.webkit.org/attachment.cgi?id=144801&action=review

> LayoutTests/fast/html/tab-order.html:5
> +        if (window.layoutTestController)
> +            layoutTestController.overridePreference('WebKitTabToLinksPreferenceKey', false);

This should be done in http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp#L205.
Comment 10 Mikhail Pozdnyakov 2012-06-04 00:57:14 PDT
Created attachment 145530 [details]
patch v4 (reset "WebKitTabToLinksPreferenceKey" to false in EFL DRT)
Comment 11 Mikhail Pozdnyakov 2012-06-04 01:00:55 PDT
Created attachment 145532 [details]
patch v4 (reset "WebKitTabToLinksPreferenceKey" to false in EFL DRT)
Comment 12 WebKit Review Bot 2012-06-04 02:28:51 PDT
Comment on attachment 145532 [details]
patch v4 (reset "WebKitTabToLinksPreferenceKey" to false in EFL DRT)

Clearing flags on attachment: 145532

Committed r119377: <http://trac.webkit.org/changeset/119377>
Comment 13 WebKit Review Bot 2012-06-04 02:28:57 PDT
All reviewed patches have been landed.  Closing bug.