Summary: | Changing selection shouldn't synchronously update editor UI components | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||
Component: | HTML Editing | Assignee: | Ryosuke Niwa <rniwa> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | barraclough, benjamin, darin, enrica, g.czajkowski, ggaren | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 127832 | ||||||||
Attachments: |
|
Description
Ryosuke Niwa
2014-02-18 20:57:46 PST
Created attachment 224592 [details]
Fixes the bug
Created attachment 224595 [details]
Fixes Windows build
Comment on attachment 224595 [details] Fixes Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=224595&action=review A couple of minor nits to consider, but r=me. > Source/WebCore/testing/Internals.cpp:1318 > + // FIXME: We shouldn't pollute the id namespace with this name. Should we file a bug to track this FIXME? > LayoutTests/platform/mac/editing/deleting/id-in-deletebutton.html:14 > + shouldBeNonNull('internals.findEditingDeleteButton(); document.getElementById("WebKit-Editing-Delete-Button")'); I don't feel like this is clearer than the original code. Committed r164401: <http://trac.webkit.org/changeset/164401> (In reply to comment #3) > (From update of attachment 224595 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=224595&action=review > > A couple of minor nits to consider, but r=me. > > > Source/WebCore/testing/Internals.cpp:1318 > > + // FIXME: We shouldn't pollute the id namespace with this name. > > Should we file a bug to track this FIXME? Filed https://bugs.webkit.org/show_bug.cgi?id=129072. > > > LayoutTests/platform/mac/editing/deleting/id-in-deletebutton.html:14 > > + shouldBeNonNull('internals.findEditingDeleteButton(); document.getElementById("WebKit-Editing-Delete-Button")'); > > I don't feel like this is clearer than the original code. This is to make expected result more self explanatory. The old expected result didn't tell us how we were finding the delete button controller at all. Removed a bogus assertion in http://trac.webkit.org/changeset/164404. Looks like this had ~1.5% performance impact on DoYouEvenBench: https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-mavericks%22%2C%22DoYouEvenBench%2FFull%3ATime%3ATotal%22%5D%5D |