1442437 - Rename background_tab_text to tab_background_text

Status: RESOLVED FIXED

(WebExtensions :: Themes, defect)

Description

[Tim Nguyen :ntim]

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 1

[Tim Nguyen :ntim]

Attachment: Bug 1442437 - Rename background_tab_text to tab_background_text.

Review commit: https://reviewboard.mozilla.org/r/224506/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/224506/

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

https://cs.chromium.org/chromium/src/chrome/browser/themes/browser_theme_pack.cc?l=232&rcl=9a5ca91778f04309cb6982697ae80a2fc89fd94b

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

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 ?

Comment 4

[Tim Nguyen :ntim]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/224506/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba49cc6e4117
Rename background_tab_text to tab_background_text. r=jaws

Comment 6

[Tim Nguyen :ntim]

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

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Liz/Ryan, flagging you to make sure this gets on your radar.

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

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!

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Marking as a blocker since jaws mentioned it as such and I want to make sure it lands for 59.

Comment 10

[Tim Nguyen :ntim]

Thank you Liz!

Ni? myself to provide a patch that applies on release sometime tomorrow.

Comment 11

[Andreea Pavel [:apavel]]

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/72b4a7b1f088 (FIREFOX_59b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/f26798687fb1

Comment 13

[Tim Nguyen :ntim]

Thanks Ryan!

Comment 14

[Bogdan Maris, Desktop Test Engineering]

Based on comment 6, marking as qe-verify-

Comment 15

[Will Bamberg [:wbamberg]]

: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.