Closed Bug 1262332 Opened 8 years ago Closed 8 years ago

[e10s] drop-down list do not appear in the right place

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
e10s m9+ ---
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: over68, Assigned: enndeakin)

References

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

Steps to reproduce:

1. Go to https://dl.dropboxusercontent.com/u/95157096/85f61cf7/18sr2y1e1j.htm.
2. Open the <select> element.


Actual results:

drop-down list appears far below the <select> element.

Screenshot https://dl.dropboxusercontent.com/u/95157096/85f61cf7/d1adxu4chw.png
Flags: needinfo?(alice0775)
Attached file reduced html
Flags: needinfo?(alice0775)
tracking-e10s: --- → ?
Regression window
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=39a90347ea46&tochange=0d71b60afe52

Regressed by: Bug 976772
Blocks: 976772
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: 48 Branch → Trunk
Component: General → Layout: Form Controls
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee: nobody → felipc
Status: NEW → ASSIGNED
Assignee: felipc → enndeakin
Attachment #8742947 - Flags: review?(tnikkel)
Comment on attachment 8742947 [details] [diff] [review]
Handle transforms in nsGlobalWindow::GetInnerScreenRect

Are we sure we want to do this? It makes sense for translations, and even scales. But anything else it might be confusing. Good thing we seem to be the only ones who have mozInnerScreenX/Y otherwise there'd be bigger compat worries.
What would you suggest instead?
Flags: needinfo?(tnikkel)
No, but I don't know how e10s positions its select dropdowns.
Flags: needinfo?(tnikkel)
Comment on attachment 8742947 [details] [diff] [review]
Handle transforms in nsGlobalWindow::GetInnerScreenRect

Since I don't know how e10s select position works I can't suggest any alternate approaches.
Attachment #8742947 - Flags: review?(tnikkel) → review+
The screen rectangle of the select button is calculated by the function getElementBoundingScreenRect in toolkit/modules/BrowserUtils.jsm  This is not the rectangle where the popup would appear but is instead the area occupied by the select button. It calculates this by adding its getBoundingClientRect to the mozInnerScreenX/Y of the enclosing window. The bug occurs when this window is in a frame that is transformed. 

This rectangle is then sent to the parent process by showDropDown in SelectContentHelper.jsm  The parent process receives this rectangle and opens a menupopup anchored at that rectangle.

Does that help explain things?
Flags: needinfo?(tnikkel)
Can you use getBoundingClientRect on the iframe of the enclosing window (instead of mozInnerScreenX/Y)? getBoundingClientRect takes transform into account I believe.
Flags: needinfo?(tnikkel)
This bug also occurs with the autocomplete.

See https://dl.dropboxusercontent.com/u/95157096/85f61cf7/te75pquroh.png
(In reply to Timothy Nikkel (:tnikkel) from comment #10)
> Can you use getBoundingClientRect on the iframe of the enclosing window
> (instead of mozInnerScreenX/Y)? getBoundingClientRect takes transform into
> account I believe.

We need screen coordinates.
Use getBoundingClientRect until you get to the top level document, then use mozInnerScreenX/Y, as the top level document can't be transformed.
So that doesn't include the size of the frame's borders. I'm not sure if it is easier to do it that way. Maybe we should just add a variation of mozInnerScreenX/Y that includes transforms?

After more testing, I notice that non-e10s select elements don't handle all types of transforms either. It seems to only handle translate, but not scale.
This version doesn't change the mozInnerScreenX/Y implementation. It also adds a test.

The offsetToTopLevelWindow function appears to be doing almost the same thing so I combined them. It's caller in FormSubmitObserver.jsm seems to want to do the same kind of thing here although it isn't correct when borders/padding exist on the iframe. I merged the two implementations. I suspect that it could be updated to use openPopupAtScreenRect which didn't exist when it was implemented.
Blocks: 1264061
Comment on attachment 8749732 [details] [diff] [review]
Handle transforms in BrowserUtils

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

This seems to work, although the test times out on Mac on the last step. That doesn't have anything to do with this fix so this I think can be reviewed now and I can continue to investigate the test issue.
Attachment #8749732 - Flags: review?(felipc)
This version is identical except that the test waits for the repaint in the frame to occur which fixes the test.
Attachment #8742947 - Attachment is obsolete: true
Attachment #8749732 - Attachment is obsolete: true
Attachment #8749732 - Flags: review?(felipc)
Attachment #8750924 - Flags: review?(felipc)
Attachment #8750924 - Flags: review?(felipc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/dddeac9ffc39e1a42b3ce866b6edd0b419be5c32
Bug 1262332, fix select popup and invalid form popup position when inside a transformed frame, r=felipe
https://hg.mozilla.org/mozilla-central/rev/dddeac9ffc39
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
should we uplift this to 48?
Flags: needinfo?(enndeakin)
Comment on attachment 8750924 [details] [diff] [review]
Handle transforms in BrowserUtils, v2

Approval Request Comment
[Feature/regressing bug #]: none
[User impact if declined]: select popups and invalid form popups appear in the wrong place in a transformed iframe, e10s only
[Describe test coverage new/current, TreeHerder]: has automated tests
[Risks and why]: minor, just improved some positioning
[String/UUID change made/needed]: none
Flags: needinfo?(enndeakin)
Attachment #8750924 - Flags: approval-mozilla-aurora?
Comment on attachment 8750924 [details] [diff] [review]
Handle transforms in BrowserUtils, v2

e10s improvement, taking it
Attachment #8750924 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.