Inspect the page: http://www.w3schools.com/cssref/tryit.asp?filename=trycss_sel_firstletter We can see each paragraph has two accessibility children, both are the same text. <rdar://problem/27220338>
<rdar://problem/27519396>
Created attachment 284463 [details] patch
Comment on attachment 284463 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=284463&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3144 > + if (obj->renderer() && obj->renderer()->isInlineElementContinuation()) shouldn't nextSibling take care of this case?
(In reply to comment #3) > Comment on attachment 284463 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284463&action=review > > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3144 > > + if (obj->renderer() && obj->renderer()->isInlineElementContinuation()) > > shouldn't nextSibling take care of this case? I think this case is happened in the firstChild() iteration.
Created attachment 284464 [details] patch Moved the check to firstChild()
Comment on attachment 284464 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=284464&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:226 > + // Sometimes with CSS first-letter selector, we were adding the same text node twice. sorry to be a jerk, but this seems like it should be in firstChildConsideringContinuation shouldn't it also if firstChild() returns null, how will nextSibling() be called
(In reply to comment #6) > Comment on attachment 284464 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284464&action=review > > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:226 > > + // Sometimes with CSS first-letter selector, we were adding the same text node twice. > > sorry to be a jerk, but this seems like it should be in > firstChildConsideringContinuation shouldn't it > > also if firstChild() returns null, how will nextSibling() be called I think in this case we don't want to add children for the inline element continuation. The real node will be handled in its parent level.
Created attachment 284467 [details] patch review comments
(In reply to comment #8) > Created attachment 284467 [details] > patch > > review comments Actually I'm not sure if putting this in firstChild() would affect other stuff or not, it passed all the test on my machine. But if we put the check in addChildren() we know for sure that this is the case we don't want to add it as a child.
Comment on attachment 284467 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=284467&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:189 > + // Sometimes with CSS first-letter selector, we were adding the same text node twice. comment is worded a bit strangely Can you change it to say why we don't want to process an inlineElementCont() as the firstChild (the CSS first letter would be noted as an example of that case)
Created attachment 284502 [details] Patch Updated the comment and added an anonymous check so that we won't affect other cases.
Comment on attachment 284502 [details] Patch Thanks. Much better
(In reply to comment #12) > Comment on attachment 284502 [details] > Patch > > Thanks. Much better R+ assuming all tests pass
(In reply to comment #13) > (In reply to comment #12) > > Comment on attachment 284502 [details] > > Patch > > > > Thanks. Much better > > R+ assuming all tests pass Thanks will commit when all tests have passed.
Comment on attachment 284502 [details] Patch Clearing flags on attachment: 284502 Committed r203694: <http://trac.webkit.org/changeset/203694>
All reviewed patches have been landed. Closing bug.