Closed Bug 1500424 Opened 6 years ago Closed 6 years ago

Remove toolbar-drag (browser / customizable UI version)

Categories

(Firefox :: General, task, P3)

task

Tracking

()

RESOLVED FIXED
Firefox 65
Tracking Status
firefox65 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file)

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.
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.
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.
Setting as P3 for now, please feel free to update
Priority: -- → P3
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: nobody → bgrinstead
Status: NEW → ASSIGNED
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)
Blocks: 1507863
Pushed by bgrinstead@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ae7984aa4803 Remove customizable ui toolbar-drag binding;r=Gijs
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)
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)
(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)
It doesn't drag before and now.
Flags: needinfo?(richard.marti)
So is there some work needed in TB? If so, Richard, please file a bug.
(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.
(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.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Flags: needinfo?(arshdkhn1)
Flags: needinfo?(mkmelin+mozilla)
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: