Bug 51308

Summary: no-caret-repaint-in-non-content-editable-element.html does not need to disable CaretBrowsing
Product: WebKit Reporter: Antonio Gomes <tonikitoo>
Component: Tools / TestsAssignee: Antonio Gomes <tonikitoo>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 51372    
Bug Blocks:    
Attachments:
Description Flags
patch v1 (r74349, r=xan) none

Description Antonio Gomes 2010-12-19 14:10:33 PST
no-caret-repaint-in-non-content-editable-element.html is the only test that explicitly disables caret browsing.

...
if (window.layoutTestController)
    layoutTestController.overridePreference("WebKitEnableCaretBrowsing", false);
...

However, this is a useless statement, since all DRT that support testing caret browsing (gtk, qt anc chromium) have it reset by default to FALSE before each test execution. On Mac and Windows, DRT that do not support testing caret browsing, it is obviously also FALSE by default.

I propose removing it.

Patch coming ...
Comment 1 Antonio Gomes 2010-12-19 14:15:31 PST
Created attachment 76961 [details]
patch v1 (r74349, r=xan)
Comment 2 Xan Lopez 2010-12-20 07:45:57 PST
Comment on attachment 76961 [details]
patch v1 (r74349, r=xan)

Makes sense to me.
Comment 3 Antonio Gomes 2010-12-20 08:15:16 PST
Comment on attachment 76961 [details]
patch v1 (r74349, r=xan)

Clearing flags on attachment: 76961

Committed r74349: <http://trac.webkit.org/changeset/74349>
Comment 4 Antonio Gomes 2010-12-20 20:46:25 PST
After talking to Daniel Bates, the original author of this test, we both agreed that the statement has no affect as DRTs are today (since caret browsing it reset before each test execution). However, explicitly setting caret browsing to DISABLED ensures that the test will run under the circumstance it needs to catch the bug: caret browsing is DISABLED.

We decided to put this code back in, and he will add a more descriptive comment saying why it should be there.

See bug 51372