Replace screenX/screenY in boxObject with properties on XULElement

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P3
normal
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: enndeakin, Assigned: enndeakin)

Tracking

(Blocks 1 bug)

unspecified
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

4 months ago

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

Assignee

Updated

4 months ago
Blocks: 1519948
No longer depends on: 1519948
Assignee

Updated

4 months ago
Priority: -- → P3
Assignee

Updated

4 months ago
Attachment #9036403 - Flags: review?(bzbarsky)
Assignee

Updated

4 months ago
Attachment #9036404 - Flags: review?(paolo.mozmail)
Assignee

Updated

4 months ago
Attachment #9036405 - Flags: review?(paolo.mozmail)
Attachment #9036403 - Flags: review?(bzbarsky) → review+

Comment 4

4 months ago
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+

Updated

4 months ago
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+

Comment 6

3 months ago
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
Assignee

Comment 7

3 months ago

I filed bug 1528373 on the issue in comment 5.

Comment 8

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Updated

3 months ago
Depends on: 1533720
You need to log in before you can comment on or make changes to this bug.