Closed Bug 1519952 Opened 2 years ago Closed 2 years ago

Replace screenX/screenY in boxObject with properties on XULElement

Categories

(Core :: XUL, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: enndeakin, Assigned: enndeakin)

References

Details

Attachments

(3 files)

Only two properties on BoxObject can't be retrieved easily via other means, screenX and screenY. Instead, move these properties to XULElement.

Blocks: 1519948
No longer depends on: 1519948
Priority: -- → P3
Attachment #9036403 - Flags: review?(bzbarsky)
Attachment #9036404 - Flags: review?(paolo.mozmail)
Attachment #9036405 - Flags: review?(paolo.mozmail)
Attachment #9036403 - Flags: review?(bzbarsky) → review+
Comment on attachment 9036404 [details] [diff] [review]
Part 2 - replace calls to boxObject.screenX(Y) with element.screenX(Y)

Review of attachment 9036404 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, thanks!
Attachment #9036404 - Flags: review?(paolo.mozmail) → review+
Attachment #9036405 - Flags: review?(paolo.mozmail) → review?(bzbarsky)
Comment on attachment 9036405 [details] [diff] [review]
Part 3 - get the screen coordinates from the frame instead of the box object in the tooltip listener

I guess this preserves the current behavior, but that behavior seems kinda wacky for at least two reasons:

1)  If it wants client coords (which I suspect it does, given the call to GetCellAt using those numbers), then why is it messing around with screen coords at all?  I guess the event's client coords might be relative to a different document?

2)  If we want client coords, we should probably be using the screen coords of the presshell's root frame, not the primary frame of the documentElement.  The two can be quite different if the documentElement is given nonzero margins or is positioned or whatnot....

r=me on this, but might be worth filing a followup on those issues.
Attachment #9036405 - Flags: review?(bzbarsky) → review+
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/41da7f72eb6e
add screenX and screenY properties to XULElement in order to replace places that currently get the screen coordinates of an element via the box object, r=bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/b980d74e702d
replace calls to boxobject screenX and screenY with the equivalent on XULElement, r=paolo
https://hg.mozilla.org/integration/mozilla-inbound/rev/4414af4d7368
don't use the boxobject screenX and screenY properties in the tooltip listener, instead get the screen coordinates from the frame directly, r=bz

I filed bug 1528373 on the issue in comment 5.

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Depends on: 1533720
You need to log in before you can comment on or make changes to this bug.