Bug 132876 – AX: Hit testing not accounting for top content inset.

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Samuel White]

WKAccessibilityWebPageObjectMac::accessibilityHitTest: is not accounting for top content inset when translating screen coords to WebKit coords.

[Samuel White]

<rdar://problem/16777273>

[Samuel White]

Created attachment 231396 [details]
Patch.

Any thoughts on testing this one? Not seeing a way to force DRT or TestRunner to HAVE a top content inset.

[WebKit Commit Bot]

Attachment 231396 [details] did not pass style-queue:


ERROR: Source/WebKit2/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectMac.mm:202:  The parameter name "page" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[chris fleizach]

Comment on attachment 231396 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=231396&action=review

> Source/WebKit2/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectMac.mm:203
> +        convertedPoint.move(0, -page->topContentInset());

is there only ever a Y contentInset? or is there also a x content inset we should account for

[Samuel White]

(In reply to comment #4)
> (From update of attachment 231396 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231396&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectMac.mm:203
> > +        convertedPoint.move(0, -page->topContentInset());
> 
> is there only ever a Y contentInset? or is there also a x content inset we should account for

That's a great question. The bug originated from a y-axis inset issue only and the exposed methods such as "topContentInset" have no matching bottom/left/right equivalents.

I think we should focus this bug on just the top inset issue since it's the only manifestation we have thus far.

[chris fleizach]

Comment on attachment 231396 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=231396&action=review

>>> Source/WebKit2/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectMac.mm:203
>>> +        convertedPoint.move(0, -page->topContentInset());
>> 
>> is there only ever a Y contentInset? or is there also a x content inset we should account for
> 
> That's a great question. The bug originated from a y-axis inset issue only and the exposed methods such as "topContentInset" have no matching bottom/left/right equivalents.
> 
> I think we should focus this bug on just the top inset issue since it's the only manifestation we have thus far.

as long as there are no equivalent contentInset parameters, this seems ok

[Samuel White]

(In reply to comment #6)
> (From update of attachment 231396 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231396&action=review
> 
> >>> Source/WebKit2/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectMac.mm:203
> >>> +        convertedPoint.move(0, -page->topContentInset());
> >> 
> >> is there only ever a Y contentInset? or is there also a x content inset we should account for
> > 
> > That's a great question. The bug originated from a y-axis inset issue only and the exposed methods such as "topContentInset" have no matching bottom/left/right equivalents.
> > 
> > I think we should focus this bug on just the top inset issue since it's the only manifestation we have thus far.
> 
> as long as there are no equivalent contentInset parameters, this seems ok

Thanks. I'll file a bug for the invalid style failure.

[Samuel White]

style failure bug:

https://bugs.webkit.org/show_bug.cgi?id=132887

[WebKit Commit Bot]

Comment on attachment 231396 [details]
Patch.

Clearing flags on attachment: 231396

Committed r168745: <http://trac.webkit.org/changeset/168745>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.