[webkit-reviews] review denied: [Bug 98357] [GTK] accessibility/aria-readonly.html is failing : [Attachment 179254] Patch

[bugzilla-daemon at webkit.org]

chris fleizach <cfleizach at apple.com> has denied Joanmarie Diggs (irc: joanie)
<jdiggs at igalia.com>'s request for review:
Bug 98357: [GTK] accessibility/aria-readonly.html is failing
https://bugs.webkit.org/show_bug.cgi?id=98357

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

------- Additional Comments from chris fleizach <cfleizach at apple.com>
ok, looked at the code.

i see that in AccessibilityRenderObject::canSetValueAttribute we're already
checking for aria_readonly == true, which looks like it should be the entry
point for this attribute. I also see that we want to check aria_readonly before
everything else, so it's a little tricky to only put it in AXNodeObject, and
obviously we shouldn't duplicate that check.

So, I think maybe we should remove the isReadOnly() method and just use
canSetAttributeValue() and move all that logic (and the method) into
AXNodeObject. 
I don't see anything that absolutely requires being inside of AXRenderObject.

That way we can implement the aria_readonly stuff first.

we also need a new test (or modify aria-readonly.html) to test the
aria-readonly=false case which will (after your patch) override native
semantics

thanks!