1483167 - Copy Link and Send Tab to Device page action button tooltips say "coypURL-title" and "sendToDevice-title"

Status: VERIFIED FIXED

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

Description

[Dão Gottwald [:dao]]

In my main use profile I moved Copy Link and Send Tab to Device to be permanently visible in the address bar, and for some weeks I've been getting "coypURL-title" and "sendToDevice-title" as the tooltip. Not sure what else I did to get to this state; I can't seem to reproduce this in a new profile.

Comment 1

[Mike de Boer [:mikedeboer]]

Perhaps you have a pref set that's related to Fluent? (I don't know whether that exists, just the first thing that came to mind  when I saw this fly by)

Comment 2

[Dão Gottwald [:dao]]

(In reply to Mike de Boer [:mikedeboer] from comment #1)
> Perhaps you have a pref set that's related to Fluent?

I don't as far as I know.

Comment 3

[Drew Willcoxon :adw]

I think I broke this with bug 1221539, which added lazy initialization for actions in the panel.

The titles/tooltips of some of these built-in actions are fetched indirectly off of attributes on the panel.  copyURL-title and sendToDevice-title are two of those attribute names.

tooltiptext is set in two places.  When you choose "Add to Address Bar," here: https://hg.mozilla.org/mozilla-central/annotate/4e56a2f51ad739ca52046723448f3129a58f1666/browser/base/content/browser-pageActions.js#l469

For new windows that should have actions in the urlbar right when they open, tooltiptext gets set via takeActionTitleFromPanel (which ends up calling updateAction, which calls _updateActionTitle), which gets called by an onPlacedInPanel callback for each action, for example: https://hg.mozilla.org/mozilla-central/annotate/4e56a2f51ad739ca52046723448f3129a58f1666/browser/base/content/browser-pageActions.js#l982

This bug only happens for new windows, so the problem is the second case.  Before bug 1221539, copyURL.onPlacedInPanel would have been called right when the window opened, but now it's only called the first time the panel opens.  So if you reproduce this bug but then open the panel, the tooltiptext gets fixed.

To fix this, there's another callback called onBeforePlacedInWindow that we could maybe use, but we should be careful not to break the lazy initialization of actions in the panel that bug 1221539 added.

Comment 4

[:Gijs (he/him)]

Given this is a regression and it's almost primary UI, I'm not sure P3 is best here. I know time for 63 nightly is short at this point, but IMO we should treat this as at least P2. Hopefully we can land a fix in the 64 cycle and uplift it. Drew, is this something you could look at or are you busy with other stuff?

Comment 5

[Drew Willcoxon :adw]

Attachment: Bug 1483167 - Copy Link and Send Tab to Device page action button tooltips say "coypURL-title" and "sendToDevice-title"

Replacing onPlacedInPanel with onBeforePlacedInWindow is the right fix.

Comment 6

[:Gijs (he/him)]

Comment on attachment 9003317 [details]
Bug 1483167 - Copy Link and Send Tab to Device page action button tooltips say "coypURL-title" and "sendToDevice-title"

:Gijs (he/him) has approved the revision.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68654fc447f8
Copy Link and Send Tab to Device page action button tooltips say "coypURL-title" and "sendToDevice-title" r=Gijs

Comment 8

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/68654fc447f8

Comment 10

[Ryan VanderMeulen [:RyanVM]]

We're building Fx62 RC builds at this point, so I think it's a bit late for uplift there. OTOH, I'm setting this to fix-optional to keep it on the radar as a possible dot release ride-along.

Comment 11

[Cristian Craciun]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:63.0) Gecko/20100101 Firefox/63.0
Build ID: 20180904100126

Verified as fixed on the latest Nightly build. 
Tested on macOS, Windows and Ubuntu. 

Note: If this issue will be fixed in Beta (62) please add back the qe-verify+ tag because I've removed it.

Comment 12

[:Gijs (he/him)]

Comment on attachment 9003317 [details]
Bug 1483167 - Copy Link and Send Tab to Device page action button tooltips say "coypURL-title" and "sendToDevice-title"

Approval Request Comment
[Feature/Bug causing the regression]: bug 1221539
[User impact if declined]: broken tooltips for page action items put in the URL bar.
[Is this code covered by automated tests?]: generally yes, not this specific bit
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: see comment #0 / comment 11.
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: relatively small + straightforward patch, already verified, js-only.
[String changes made/needed]: no

Comment 13

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9003317 [details]
Bug 1483167 - Copy Link and Send Tab to Device page action button tooltips say "coypURL-title" and "sendToDevice-title"

fix broken tooltips, verified in 63, approved for 62.0.2

Comment 14

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-release/rev/0a1434f27587

Comment 15

[Cristian Craciun]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180920131237

Verified as fixed on the Release candidate build 62.0.2