Bug 129024 – Changing selection shouldn't synchronously update editor UI components

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

Right now, editor UI controller such as spellchecker, alternative text controller, and delete button controller
are all updated synchronously upon selection change.

We shouldn't do that at least for programatic selection changes.

[Ryosuke Niwa]

Created attachment 224592 [details]
Fixes the bug

[Ryosuke Niwa]

Created attachment 224595 [details]
Fixes Windows build

[Brent Fulgham]

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.

[Ryosuke Niwa]

Committed r164401: <http://trac.webkit.org/changeset/164401>

[Ryosuke Niwa]

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

[Ryosuke Niwa]

Removed a bogus assertion in http://trac.webkit.org/changeset/164404.

[Ryosuke Niwa]

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