1786293 - Firefox View button missing on first run with browser.tabs.firefox-view = true

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P1)

Description

[Dão Gottwald [:dao]]

With browser.tabs.firefox-view set to true by default, the first run with a new profile has the Firefox View button missing. The button appears in subsequent runs with that profile. browser_firefoxview_tab.js has run into this problem, and bug 1771311 got backed out for it. I've tried to debug this but got kind of lost in the weeds of CustomizableUI. Still trying to make sense of this.

Comment 1

[Dão Gottwald [:dao]]

Attachment: CustomizableUI log of first run

Comment 2

[Dão Gottwald [:dao]]

Attachment: CustomizableUI log of second run

Comment 3

[Dão Gottwald [:dao]]

Attachment: Diff between 1st and 2nd run logs

Comment 4

[Dão Gottwald [:dao]]

Attachment: Diff between 1st and 2nd run logs

Comment 5

[Sam Foster [:sfoster] (he/him)]

After a bit of digging and talking with :jaws, I think the answer here is to add the firefox-view-button to the markup in navigator-toolbox.inc.xhtml - which we want for performance in the default case. This should mean that everything is already where it should be and we don't need to mess around with the cache-dirtying. And we don't need to "ifdef nightly" that now we're in 106.

In the first-run case where firefox-view is disabled, we might need to check that restoring to defaults correctly reflects the policy/initial-pref values but that should probably be a follow-up bug.

Comment 6

[Dão Gottwald [:dao]]

That sounds more like a workaround than fixing the underlying issue. :/ I wonder how this works for save-to-pocket-button which isn't in the markup? Could we do the same?

I guess we'd want to go the markup route anyway if there are performance reasons, although I'm a bit skeptical that there's going to be measurable impact of building the DOM nodes dynamically if it happens before the first paint... perhaps I'm too optimistic. I seem to recall that at some point we were actually going to move more widgets from markup to the JSM. Is performance the reason why we ended up not doing that?

Comment 7

[Dão Gottwald [:dao]]

I'll just go ahead and do what Sam and Jared discussed so we can finally flip the pref for Nightly.

Comment 8

[Dão Gottwald [:dao]]

Attachment: Bug 1786293 - Move Firefox View button from CustomizableWidgets to markup. r=sfoster

Comment 9

[Sam Foster [:sfoster] (he/him)]

(In reply to Dão Gottwald [::dao] from comment #6)

That sounds more like a workaround than fixing the underlying issue. :/ I wonder how this works for save-to-pocket-button which isn't in the markup? Could we do the same?

Yeah save-to-pocket does seem to be an oddball, I don't think its a useful pattern to follow. I agree that it shouldn't be strictly necessary to have elements in the markup for this to work, but in this specific case I think the issue is is that it isn't clear this is the design and how its intended to work - for better or worse.

I seem to recall that at some point we were actually going to move more widgets from markup to the JSM. Is performance the reason why we ended up not doing that?

Yeah I believe so, though :jaws and :gijs would have more context.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/114f8b2579ef
Move Firefox View button from CustomizableWidgets to markup. r=Gijs

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/114f8b2579ef

Comment 12

[Alex Finder]

== Change summary for alert #35232 (as of Mon, 29 Aug 2022 08:52:31 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
3%	google-mail LastVisualChange	linux1804-64-shippable-qr	cold fission webrender	4,350.00 -> 4,206.67

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=35232

Comment 13

[:Gijs (he/him)]

(In reply to Alex Finder from comment #12)

== Change summary for alert #35232 (as of Mon, 29 Aug 2022 08:52:31 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
3%	google-mail LastVisualChange	linux1804-64-shippable-qr	cold fission webrender	4,350.00 -> 4,206.67

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=35232

I don't think this change is related to this patch (which only changed frontend code, no web rendering changes)

Comment 14

[Alexandru Trif, Desktop QA [:atrif]]

Reproduced the issue with Firefox (20220822190304) by opening the build with browser.tabs.firefox-view set to true by default on Windows 10x64. The Firefox View is not displayed on the first run.
The issue is no longer reproducible with Firefox 106.0b8 on Windows 10x64, macOS 10.15 and Ubuntu 20.04. The Firefox View buttons is displayed on the first run.