Closed Bug 1827910 Opened 2 years ago Closed 2 years ago

Deprecate browser_style in MV3

Categories

(WebExtensions :: Frontend, enhancement, P2)

enhancement

Tracking

(firefox114 fixed)

RESOLVED FIXED
114 Branch
Tracking Status
firefox114 --- fixed

People

(Reporter: robwu, Assigned: robwu)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [wecg][addons-jira])

Attachments

(1 file)

The browser_style feature is supposed to allow extensions to inherit the "native browser style" from the browser. In practice, the extension.css stylesheet is unmaintained and does not keep up with changes from the browser (e.g. bug 1421652), partly because any change to the stylesheet has potential to affect many others.

Besides not living up to the promise of matching the browser style, the stylesheet also increases the complexity of supporting dark theme (e.g. without the browser_style dark theme support would be as easy as adding <meta name="color-scheme" content="light dark">, now some additional CSS is needed to undo the browser_style, e.g. as seen in this example).

We should deprecate browser_style. MV3 was released relatively recently (bug 1801291), so deprecating sooner than later would minimize impact. Chrome's counterpart (chrome_style) was also dropped from MV3 (https://crbug.com/973157).

browser_style defaults to true for the following, which means that every extension will have the styles by default:

browser_style is available and defaults to false for the following, which means that these pages are only affected if the extension opted in with "browser_style": true.

Global styles

The only "global" styles are as follows (with annotations added by me). Out of all CSS rules, box-sizing: border-box and display: flex have the most potential to alter the layout significantly.

html,
body {
  background: transparent; /* already default (Canvas) */
  box-sizing: border-box; /* defaults to padding-box, may affect some layout */
  color: #222426; /* defaults to #00000 (black) (CanvasText) */
  cursor: default; /* defaults to auto */
  display: flex; /* defaults to block; affects layout */
  flex-direction: column; /* defaults to row; affects layout */
  font: caption; /* affects font; but not fixed in practice */
  margin: 0; /* overrides body { margin: 8px; } */
  padding: 0; /* already default */
  user-select: none; /* user unfriendly; defaults to user-select: auto */
}

body * {
  box-sizing: border-box; /* affects layout */
  text-align: start; /* already default */
}

.browser-style and other class names

extension.css also enables styling for elements with the .browser-style class name and some other class names starting with panel:

Whiteboard: [wecg] → [wecg][addons-jira]
Assignee: nobody → rob
Status: NEW → ASSIGNED
See Also: → 1398466
Severity: -- → N/A
Priority: -- → P2

Editing title to more accurately reflect that the initial step is to deprecate.

My current plan is as follows:

  • this bug: Firefox 114 (or 113 if approved): no behavioral changes, only log messages.
  • bug 1830710: Firefox 115: change defaults for options_ui.browser_style and sidebar_action.browser_style from true to false (in MV3).
  • bug 1830711: Future: drop support for browser_style: true in MV3, by default.
  • bug 1830712: Far future: remove code and prefs supporting browser_style: true in MV3.

The behavior can be controlled through the following preferences:

  • extensions.browser_style_mv3.supported ; initially true, ultimately false (in bug 1830711).
  • extensions.browser_style_mv3.same_as_mv2 ; initially true, then false (in bug 1830710).
Summary: Deprecate and remove browser_style from MV3 → Deprecate browser_style in MV3

This patch has no observable changes, other than printing deprecation
messages when browser_style is effectively true in MV3.

This patch does include the full logic for all stages of the deprecation
process behind prefs, which will follow the schedule described at:
https://bugzilla.mozilla.org/show_bug.cgi?id=1827910#c1.

All combinations of these prefs are fully covered by unit tests in
toolkit/components/extensions/test/xpcshell/test_ext_browser_style_deprecation.js
The next test tasks confirm the behavior of the current patch:

  • browser_style_never_deprecated_in_MV2
  • supported_with_browser_style_false
  • supported_with_browser_style_true
  • supported_with_mv2_defaults
See Also: → 1816960
Pushed by rob@robwu.nl: https://hg.mozilla.org/integration/autoland/rev/afc12be87873 Show deprecation warnings for browser_style in MV3 r=willdurand

dev-doc-needed:

See initial comment for details and comment 1 for the deprecation plan.

Discourage the use of browser_style and the reliance on it in:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/user_interface/Browser_styles

If one really depends on the browser_style: true styles, they can bundle the following stylesheet with their extension:

While probably not interesting, note that there are macOS-specific styles for the .browser-style class; if an extension is interested in these as well:

Keywords: dev-doc-needed
Regressions: 1831323

Backed out for causing xpcshell failures in test_ext_browser_style_deprecation.js

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_browser_style_deprecation.js | browser_style_never_deprecated_in_MV2 - [browser_style_never_deprecated_in_MV2 : 62] Got expected warnings for MV2 extension with browser_style:true. - ["Reading manifest: Warning processing sidebar_action: An unexpected property was found in the WebExtension manifest."] deepEqual []
Flags: needinfo?(rob)
No longer regressions: 1831323

Test failed on Android because the sidebar_action is desktop-only. I'm going to reland with the adjusted test: https://phabricator.services.mozilla.com/D176811?vs=712282&id=712402#diff-change-oy6TmTwIFdFy

Flags: needinfo?(rob)
Pushed by rob@robwu.nl: https://hg.mozilla.org/integration/autoland/rev/11d90368805b Show deprecation warnings for browser_style in MV3 r=willdurand

Test failed again on Android because an UnrecognizedProperty is not omitted, but included (without validation), causing the runtime check to run validation for sidebar_action despite the build not supporting sidebar_action. I've fixed this by wrapping the runtime check in an additional condition: https://phabricator.services.mozilla.com/D176811?vs=712407&id=712502#C6353775NL1506

I have also filed bug to follow up on this aspect of UnrecognizedProperty, under the second class of problems in bug 1831417.

Flags: needinfo?(rob)
Pushed by rob@robwu.nl: https://hg.mozilla.org/integration/autoland/rev/43be13c1efa4 Show deprecation warnings for browser_style in MV3 r=willdurand
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 114 Branch

Documentation updates in:

Depends on: 1832578
Regressions: 1831709
See Also: → 1873024
See Also: → 1593355
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: