1500424 - Remove toolbar-drag (browser / customizable UI version)

Status: RESOLVED FIXED

(Firefox :: General, task, P3)

Description

[Brian Grinstead [:bgrins]]

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.

Comment 1

[Brian Grinstead [:bgrins]]

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

[Dão Gottwald [:dao]]

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.

Comment 3

[Drew Willcoxon :adw]

Setting as P3 for now, please feel free to update

Comment 4

[Brian Grinstead [:bgrins]]

Attachment: Bug 1500424 - Remove customizable ui toolbar-drag binding;r=Gijs

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.

Comment 5

[Brian Grinstead [:bgrins]]

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.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae7984aa4803
Remove customizable ui toolbar-drag binding;r=Gijs

Comment 7

[Jorg K (CEST = GMT+2)]

Thanks Brian. I'll forward the NI to some people who know better than me.

Comment 8

[Richard Marti (:Paenglab)]

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.

Comment 9

[Brian Grinstead [:bgrins]]

(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`.

Comment 10

[Richard Marti (:Paenglab)]

It doesn't drag before and now.

Comment 11

[Jorg K (CEST = GMT+2)]

So is there some work needed in TB? If so, Richard, please file a bug.

Comment 12

[Richard Marti (:Paenglab)]

(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

[Tim Nguyen :ntim]

(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

[Andreea Pavel [:apavel]]

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