Closed
Bug 1500424
Opened 6 years ago
Closed 6 years ago
Remove toolbar-drag (browser / customizable UI version)
Categories
(Firefox :: General, task, P3)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: bgrins, Assigned: bgrins)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
This is linux-only: https://searchfox.org/mozilla-central/rev/c56977420df7a1b692ce0f7e499ddb364d9fd7b2/browser/themes/linux/browser.css#489-498. Note that `moz-menubar-drag` is used in GTK, so for those elements we could maybe use matchMedia.
From https://bugzilla.mozilla.org/show_bug.cgi?id=1473311#c0:
> Move the window dragging code to the extant windowdragging jsm, and invoke it from browser.js and https://dxr.mozilla.org/mozilla-central/source/browser/components/places/content/places.js (possibly later than we do now, which should be fine and a minor perf win).
An alternative would be to set up a Custom Element for `toolbar` that checks attrs / media queries and sets this up if necessary. If we did that we could tackle the toolkit version at the same time and not explicitly set up drag for the callers in browser/. The problem there is that we'd have to wait until we removed the XBL binding for CUI toolbar, since we can't have both CE and XBL attached.
Assignee | ||
Comment 1•6 years ago
|
||
Two other thoughts:
1) We could rewrite consumers from [type="menubar"] to [is="toolbar-draggable"] or similar, and then register a customized built-in CE (as in https://searchfox.org/mozilla-central/rev/9cb3e241502a2d47e2d5057ca771324a446b6695/toolkit/components/printing/content/printPreviewToolbar.js#10). I think the browser consumers don't even set `type="menubar"` at the moment, so we could add `[is]` to those as well.
2) We could change the window dragging util to bind handlers to the document and use event delegation to look for the right type of toolbar, rather than wiring it up to each element individually.
Comment 2•6 years ago
|
||
Even better would be to get -moz-window-dragging to work on GTK -- then we could get rid of not only the binding but the whole JSM.
Assignee | ||
Comment 4•6 years ago
|
||
It was almost identical to the toolkit version, only missing a [customizing=true]
check to prevent drag. Since Customization only happens in browser/ we are able
to replace the toolkit version with the CUI version, and then remove the CUI version.
The `#toolbar-menubar:not([autohide="true"])` selector will fall back to the
`toolbar[type="menubar"]` selector in global.css to apply the toolkit one, so
that is removed from browser.css.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•6 years ago
|
||
Jorg, this patch slightly changes the linux-only "toolbar-drag" binding (it removes this line checking `this.getAttribute("customizing") != "true"`): https://searchfox.org/mozilla-central/rev/c0b26c40769a1e5607a1ae8be37fe64df64fc55e/toolkit/content/widgets/toolbar.xml#25. As far as we can tell, that isn't important to Firefox (since we always overrode this binding with a browser version that didn't have the check), so worth double checking if that's relevant to TB.
Flags: needinfo?(jorgk)
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ae7984aa4803
Remove customizable ui toolbar-drag binding;r=Gijs
Comment 7•6 years ago
|
||
Thanks Brian. I'll forward the NI to some people who know better than me.
Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(jorgk)
Flags: needinfo?(arshdkhn1)
Comment 8•6 years ago
|
||
I built TB with the patch and a quick check showed customizing still works as before. Dragging with the menubar TB during the customizing doesn't work as before.
Flags: needinfo?(richard.marti)
Assignee | ||
Comment 9•6 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #8)
> I built TB with the patch and a quick check showed customizing still works
> as before. Dragging with the menubar TB during the customizing doesn't work
> as before.
To confirm: it used to not drag, but now it does? That's what I would expect given that we are removing a conditional check in `mouseDownCheck`.
Flags: needinfo?(richard.marti)
Comment 11•6 years ago
|
||
So is there some work needed in TB? If so, Richard, please file a bug.
Comment 12•6 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #11)
> So is there some work needed in TB? If so, Richard, please file a bug.
Not needed as far as I see it.
Comment 13•6 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #10)
> It doesn't drag before and now.
Makes sense, TB's customization mode is a modal dialog, which prevents the other window from being dragged. Firefox doesn't use a modal, so the behaviour only matters for TB.
Comment 14•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Updated•6 years ago
|
Flags: needinfo?(arshdkhn1)
Updated•6 years ago
|
Flags: needinfo?(mkmelin+mozilla)
Updated•5 years ago
|
Type: enhancement → task
You need to log in
before you can comment on or make changes to this bug.
Description
•