Bug 87668 – [EFL] Reset "WebKitTabToLinksPreferenceKey" in DumpRenderTreeChrome::resetDefaultsToConsistentValues

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Mikhail Pozdnyakov]

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).

[Mikhail Pozdnyakov]

Created attachment 144368 [details]
patch

[Raphael Kubo da Costa (:rakuco)]

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.

[Mikhail Pozdnyakov]

Created attachment 144382 [details]
patch v2 (corrected changelog)

[Mikhail Pozdnyakov]

(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.

[Gustavo Noronha (kov)]

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?

[Mikhail Pozdnyakov]

(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.

[Mikhail Pozdnyakov]

Created attachment 144801 [details]
patch v3 (removed EFL related part)

[Alexey Proskuryakov]

I agree with Gustavo - it's wrong to rely on a test to reset preferences to original state itself. Very few tests do that.

[Ryosuke Niwa]

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.

[Mikhail Pozdnyakov]

Created attachment 145530 [details]
patch v4 (reset "WebKitTabToLinksPreferenceKey" to false in EFL DRT)

[Mikhail Pozdnyakov]

Created attachment 145532 [details]
patch v4 (reset "WebKitTabToLinksPreferenceKey" to false in EFL DRT)

[WebKit Review Bot]

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>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.