Rename background_tab_text to tab_background_text

RESOLVED FIXED in Firefox 59

Status

defect
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: ntim, Assigned: ntim)

Tracking

(Blocks 1 bug, {dev-doc-complete})

unspecified
mozilla60
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox58 unaffected, firefox59blocking fixed, firefox60+ fixed)

Details

Attachments

(1 attachment)

Assignee

Description

a year ago
I discovered recently that our chrome compatibility alias is wrong. It should be tab_background_text not background_tab_text.


Let's also uplift this change to Firefox 59 as well, to avoid exposing the wrong property name to all extensions on the release channel (which would be problematic as we'd have to support it forever).
Comment hidden (mozreview-request)
Assignee

Updated

a year ago
Assignee

Updated

a year ago
Keywords: dev-doc-needed
Comment on attachment 8955339 [details]
Bug 1442437 - Rename background_tab_text to tab_background_text.

https://reviewboard.mozilla.org/r/224506/#review230454

::: browser/components/extensions/test/browser/browser_ext_themes_icons.js:85
(Diff revision 1)
>        "images": {
>          "theme_frame": "fox.svg",
>        },
>        "colors": {
>          "frame": FRAME_COLOR,
> -        "background_tab_text": BACKGROUND_TAB_TEXT_COLOR,
> +        "tab_background_text": BACKGROUND_TAB_TEXT_COLOR,

Can you also rename BACKGROUND_TAB_TEXT_COLOR to TAB_BACKGROUND_TEXT_COLOR ?
Attachment #8955339 - Flags: review?(jaws) → review+
Comment hidden (mozreview-request)

Comment 5

a year ago
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ba49cc6e4117
Rename background_tab_text to tab_background_text. r=jaws
Assignee

Comment 6

a year ago
Comment on attachment 8955339 [details]
Bug 1442437 - Rename background_tab_text to tab_background_text.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1415872
[User impact if declined]: none for the user, but not uplifting to Firefox 59 means we'll have to support an incorrect name in extensions forever, because extensions would start using it once it reaches release.

[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not applicable
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: simple rename
[String changes made/needed]: no
Attachment #8955339 - Flags: approval-mozilla-release?
Liz/Ryan, flagging you to make sure this gets on your radar.
Flags: needinfo?(ryanvm)
Flags: needinfo?(lhenry)
Comment on attachment 8955339 [details]
Bug 1442437 - Rename background_tab_text to tab_background_text.

Fix for a potentially annoying regression in 59, let's uplift this for the RC build next Monday. Thanks for fixing the tests too!
Flags: needinfo?(lhenry)
Attachment #8955339 - Flags: approval-mozilla-release? → approval-mozilla-release+
Marking as a blocker since jaws mentioned it as such and I want to make sure it lands for 59.
blocking-fx: ? → ---
Assignee

Comment 10

a year ago
Thank you Liz!

Ni? myself to provide a patch that applies on release sometime tomorrow.
Flags: needinfo?(ntim.bugs)

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ba49cc6e4117
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Flags: needinfo?(ryanvm)
Assignee

Comment 13

a year ago
Thanks Ryan!
Flags: needinfo?(ntim.bugs)
Based on comment 6, marking as qe-verify-
Flags: qe-verify-
:ntim, I think you have updated the docs page and data for this, so if you are happy with the screenshots we can mark it dev-doc-complete.
Flags: needinfo?(ntim.bugs)
Assignee

Updated

a year ago
Flags: needinfo?(ntim.bugs)

Updated

11 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.