Closed Bug 1533958 Opened 2 years ago Closed 2 years ago

[Fission] Make <select> dropdowns work with Fission

Categories

(Toolkit :: XUL Widgets, enhancement, P2)

Product:
Toolkit
Component:
XUL Widgets
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla69
Project Flags:
Fission Milestone M3
Tracking Flags:
Tracking Status
firefox69 --- fixed

People

(Reporter: Felipe, Assigned: ablayelyfondou)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

Bug 1533958 - Make <select> works with Fission. r?NeilDeakin
47 bytes, text/x-phabricator-request
Details | Review

Make the <select> drop-downs work with out-of-process iframes. This will likely involve changes to SelectChild.jsm (to convert it to a WindowActor) and to port SelectParentHelper.jsm to its parent-side window actor.

Fission Milestone: --- → M2
Fission Milestone: M2 → M3
Assignee: nobody → ablayelyfondou
Priority: P3 → P2
Depends on: 1555138

Hey Neil, is the idea to try to get bug 1555138 fixed before this can land? Or can it land now?

Flags: needinfo?(enndeakin)
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/239751330c09
Make <select> works with Fission. r=neil

Bug 1555138 only affects fission-enabled iframes and makes the popup appear at the wrong screen offset. So better than the current state where the popup doesn't appear at all.

Flags: needinfo?(enndeakin)

From talking to rhunt, I think BrowserParent::TransformChildToParent would be helpful here for getting the correct coordinate to place the popup at.

(In reply to Arthur Iakab [arthur_iakab] from comment #5)

Backed out changeset 239751330c09 (bug 1533958) for causing browser chrome leaks on browser_event_listener.js CLOSED TREE

Backout revision https://hg.mozilla.org/integration/mozilla-inbound/rev/b1d382dd18da4460ac88849c12049fa6d1a4cee2

Failures https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&fromchange=b439d3fd8862e709eae9525791780b61ac98dbf1&selectedJob=249439400&searchStr=linux%2Cx64%2Cdebug%2Cmochitests%2Ctest-linux64%2Fdebug-mochitest-browser-chrome-e10s-2%2Cm%28bc2%29

Failed push https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=249453447&revision=239751330c09bdb7d1ea2ad2cd87685e936c0007

Failure logs https://treeherder.mozilla.org/logviewer.html#?job_id=249439400&repo=mozilla-inbound

Abdoulaye O. LY can you please take a look?

Sure, I am working on it.

From talking to rhunt, I think BrowserParent::TransformChildToParent would be helpful here for getting the correct coordinate to place the popup at.

Yes, that seems like a better idea. Are you going to provide an API for this?

Flags: needinfo?(ablayelyfondou) → needinfo?(jwatt)
Depends on: 1557448
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ced634e12fc3
Make <select> works with Fission. r=NeilDeakin
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ffde2ba5b8c6
Make <select> works with Fission. r=NeilDeakin

https://hg.mozilla.org/mozilla-central/rev/ffde2ba5b8c6

Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox69: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Flags: needinfo?(ablayelyfondou)

I thought since this bug was fixed you managed without additional API, but from our chat at the All Hands the positioning is still wrong. Can you file a new bug, needinfo me there, and note the number of that bug here?

Flags: needinfo?(jwatt) → needinfo?(ablayelyfondou)
Depends on: 1560627

This is landed since it does not regress anything but the coordinates are not correct with fission. Bug 1560627 is filed for the positioning issue.

Flags: needinfo?(ablayelyfondou)
Regressions: 1569570

We just discovered that this bug regressed RDM. The select drop-downs do not appear anymore within the RDM viewport now. So that's a P1 we need to fix.
It would be great if a work around could be found and uplifted to 69 so we don't ship a broken RDM to all our users.

The better long term solution is to fix bug 1549775 so problems like these don't occur anymore, but it will take a long time.

I think RDM broke because it used to forward the Forms:DismissedDropDown messages through it's tunnel, but this message was probably removed in favor of new Actor code.

Abdoulaye, does any idea come to mind for how we might make RDM still work with your change?

Flags: needinfo?(ablayelyfondou)

Whoops sorry for the mess introduced with this patch. I need to have a closer look at this.

Flags: needinfo?(ablayelyfondou)

@Neil and @Abdoulaye: Just for context, the regression we are experiencing is that that the RDM no longer shows the menulist on select elements. This is because devtools/client/responsive.html/index.xhtml now requires it's own xul:menulist. We could add this and include widgets.css but we are wondering if there is another way around this.

We have a partial fix in bug 1569570 but some way without adding our own xul:menulist would be great.

Flags: needinfo?(mconley)
Flags: needinfo?(mconley)
Flags: needinfo?(enndeakin)
Flags: needinfo?(ablayelyfondou)

Just found Abdoulayes response in bug 1569570 so nothing needed here.

Flags: needinfo?(enndeakin)
Flags: needinfo?(ablayelyfondou)
See Also: → 1587243
You need to log in before you can comment on or make changes to this bug.