[webkit-reviews] review requested: [Bug 106924] [GTK] GTK does not expose heading level correctly. Was: accessibility/heading-level.html is failing : [Attachment 212466] Patch proposal

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <mario at webkit.org> has asked  for review:
Bug 106924: [GTK] GTK does not expose heading level correctly. Was:
accessibility/heading-level.html is failing
https://bugs.webkit.org/show_bug.cgi?id=106924

Attachment 212466: Patch proposal
https://bugs.webkit.org/attachment.cgi?id=212466&action=review

------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
(In reply to comment #8)
> [...]
> It seems that the problem was caused by a call to coreObject->helpText() in 
> webkitAccessibleGetAttributes, in the ATK wrapper, which was causing a text
> computation under the element just for the sake of exposing the 'aria-help'
> attribute, which ended up altering the a11y tree and getting this weird
> result.
> 
> Fortunately, we can just remove that 'aria-help' mapping from there since
> it's wrong in the first place, and once we do that this tests will pass
> gracefully, assuming that the patch attached here is approved.
> 
> For the removal of the 'aria-help' mapping I filed bug 121675 and provided
> a patch there, so now setting the dependency here and moving on.

It seems that this accessibility/heading-level.html test is not crashing
anymore but just failing, since removing that call to coreObject->helpText()
from there do not cause the segfault to happen anymore in the context of this
test. So, I'm now proposing a new patch that is basically the previous one +
the removal of the test from TestExpectations file.

However, a problem related to trying to use the WebCore's accessibility API
when the render tree is not stable (needing a layout update) still remains,
since the layout test proposed with the current patch for bug 121558 still
crashes in GTK. But that's a separate issue anyway (even if it was detected
through this bug in the first place), so I'd rather keep it separate and work
on it there instead.

So, please review this patch for this specific thing. I'll keep working in that
other potential crash in bug 121558. Thanks!