Bug 51308 - no-caret-repaint-in-non-content-editable-element.html does not need to disable CaretBrowsing
Summary: no-caret-repaint-in-non-content-editable-element.html does not need to disabl...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Antonio Gomes
Depends on: 51372
  Show dependency treegraph
Reported: 2010-12-19 14:10 PST by Antonio Gomes
Modified: 2010-12-20 20:46 PST (History)
0 users

See Also:

patch v1 (r74349, r=xan) (2.65 KB, patch)
2010-12-19 14:15 PST, Antonio Gomes
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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