Bug 132876 - AX: Hit testing not accounting for top content inset.
Summary: AX: Hit testing not accounting for top content inset.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.9
: P1 Normal
Assignee: Samuel White
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-05-13 11:02 PDT by Samuel White
Modified: 2014-05-13 15:27 PDT (History)
2 users (show)

See Also:


Attachments
Patch. (1.55 KB, patch)
2014-05-13 11:09 PDT, Samuel White
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel White 2014-05-13 11:02:18 PDT
WKAccessibilityWebPageObjectMac::accessibilityHitTest: is not accounting for top content inset when translating screen coords to WebKit coords.
Comment 1 Samuel White 2014-05-13 11:03:56 PDT
<rdar://problem/16777273>
Comment 2 Samuel White 2014-05-13 11:09:53 PDT
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.
Comment 3 WebKit Commit Bot 2014-05-13 11:11:43 PDT
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.
Comment 4 chris fleizach 2014-05-13 11:45:26 PDT
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
Comment 5 Samuel White 2014-05-13 14:08:48 PDT
(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.
Comment 6 chris fleizach 2014-05-13 14:12:31 PDT
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
Comment 7 Samuel White 2014-05-13 14:55:36 PDT
(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.
Comment 8 Samuel White 2014-05-13 15:00:32 PDT
style failure bug:

https://bugs.webkit.org/show_bug.cgi?id=132887
Comment 9 WebKit Commit Bot 2014-05-13 15:27:12 PDT
Comment on attachment 231396 [details]
Patch.

Clearing flags on attachment: 231396

Committed r168745: <http://trac.webkit.org/changeset/168745>
Comment 10 WebKit Commit Bot 2014-05-13 15:27:15 PDT
All reviewed patches have been landed.  Closing bug.