Closed Bug 1483167 Opened 2 years ago Closed 2 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- wontfix
firefox62 + verified
firefox63 --- verified

People

(Reporter: dao, Assigned: adw)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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)
(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.
Component: Address Bar → Toolbars and Customization
Priority: -- → P3
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.
Blocks: 1221539
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?
Flags: needinfo?(adw)
Assignee: nobody → adw
Status: NEW → ASSIGNED
Flags: needinfo?(adw)
Replacing onPlacedInPanel with onBeforePlacedInWindow is the right fix.
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.
Attachment #9003317 - Flags: review+
Pushed by dwillcoxon@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/68654fc447f8
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Flags: qe-verify+
Duplicate of this bug: 1485348
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.
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
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
Attachment #9003317 - Flags: approval-mozilla-release?
Blocks: 1489834
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
Attachment #9003317 - Flags: approval-mozilla-release? → approval-mozilla-release+
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
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.