To reproduce: navigate to the URL in Safari, choose Edit > Find > Find, and in the Find bar, type “rule” (without the quotes). Notice how Safari reports more than 1,000 matches. This was caused by <http://trac.webkit.org/r158617>, the fix for bug 123697.
<rdar://problem/15733725>
The problem with http://trac.webkit.org/r158617 is that it changes the behavior of the low-level function Range::firstNode. I’m going to roll that part of the patch out and then see what happens to the accessibility behavior.
I just tested Safari 7.0.3 and it also shows the "more then 1,000 matches" behavior. Does that make sense given that http://trac.webkit.org/r158617 should not be there in the WebKit used by Safari 7.0.3?
(In reply to comment #3) > I just tested Safari 7.0.3 and it also shows the "more then 1,000 matches" behavior. Does that make sense given that http://trac.webkit.org/r158617 should not be there in the WebKit used by Safari 7.0.3? I think this change went into the 7.0.3 update
Yes, removing the incorrect change to Range::firstNode does indeed fix the find problem right away. But it reintroduces the problem from bug 123697. The core of the problem is trying to make elements with rendered children work the way that <img> elements work with our editing code. Unfortunately, for <img> elements, we are still stuck with the old model where we represent “before the image” as (<img>, 0) in a lot of code; parent node <img>, offset 0. Which in actual Range semantics means “inside the image”, not “before the image“. We have a long term plan to get rid of this, but since it’s still around it causes us major headaches. Here, the problem is that for the <div role="image">, (<div>, 0) can mean either “before the image” or “inside the image”. The hacks that make this almost work are getting out of hand at this point. Instead of modifying firstNode, we need to modify Position::parentAnchoredEquivalent to handle this case.
Created attachment 229236 [details] Patch
The patch is ready to go and passes all the existing tests. But before landing it we should really add a new test for the actual problem with find. That’s the only thing preventing me from putting this up for review and landing it.
Created attachment 229259 [details] Patch
Comment on attachment 229259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229259&action=review > Source/WebCore/dom/Element.cpp:2944 > + return !equalIgnoringCase(fastGetAttribute(roleAttr), "img"); Should other element subclasses that override this method check this condition as well?
(In reply to comment #9) > Should other element subclasses that override this method check this condition as well? The only element classes that override this for any reason other than to unconditionally return false are HTMLObjectElement and HTMLHRElement. I’m not sure that either of them really needs to support role="image", but I could change them to call through to the base class.
Committed r167210: <http://trac.webkit.org/changeset/167210>
This change broke a test, editing/editability/ignored-content.html either fails or asserts: http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=editing%2Feditability%2Fignored-content.html
Committed r167291: <http://trac.webkit.org/changeset/167291>
The followup fixes the problem seen on the bots/flakiness dashboard.
Confirmed, thank you!