Closed Bug 1743102 Opened 6 months ago Closed 5 months ago

Add color-scheme meta tag to remaining compatible about: pages

Categories

(Toolkit :: General, defect, P3)

Desktop
All
defect

Tracking

()

RESOLVED FIXED
97 Branch
Tracking Status
firefox97 --- fixed

People

(Reporter: Gijs, Assigned: mpj.5, Mentored)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file)

We missed a few privileged pages in bug 1742696.

Full list of about: pages:

0: "about:about"
​1: "about:addons"
​2: "about:blank"
​3: "about:blocked"
​4: "about:buildconfig"
​5: "about:cache"
​6: "about:cache-entry"
​7: "about:certerror"
​8: "about:certificate"
​9: "about:checkerboard"
​10: "about:compat"
​11: "about:config"
​12: "about:crashcontent"
​13: "about:crashes"
​14: "about:crashparent"
​15: "about:credits"
​16: "about:debugging"
​17: "about:devtools"
​18: "about:devtools-toolbox"
​19: "about:downloads"
​20: "about:framecrashed"
​21: "about:glean"
​22: "about:home"
​23: "about:httpsonlyerror"
​24: "about:ion"
​25: "about:license"
​26: "about:logins"
​27: "about:loginsimportreport"
​28: "about:logo"
​29: "about:memory"
​30: "about:mozilla"
​31: "about:neterror"
​32: "about:networking"
​33: "about:newtab"
​34: "about:performance"
​35: "about:plugins"
​36: "about:pocket-home"
​37: "about:pocket-saved"
​38: "about:pocket-signup"
​39: "about:policies"
​40: "about:preferences"
​41: "about:printpreview"
​42: "about:privatebrowsing"
​43: "about:processes"
​44: "about:profiles"
​45: "about:profiling"
​46: "about:protections"
​47: "about:reader"
​48: "about:restartrequired"
​49: "about:rights"
​50: "about:robots"
​51: "about:serviceworkers"
​52: "about:sessionrestore"
​53: "about:srcdoc"
​54: "about:studies"
​55: "about:support"
​56: "about:sync-log"
​57: "about:tabcrashed"
​58: "about:telemetry"
​59: "about:third-party"
​60: "about:unloads"
​61: "about:url-classifier"
​62: "about:webrtc"
​63: "about:welcome"
​64: "about:welcomeback"

Minus the ones we already did or don't want to add it to:

1: "about:addons"
​​6: "about:cache-entry"
​​​​11: "about:config"
​​​​16: "about:debugging"
​17: "about:devtools"
​18: "about:devtools-toolbox"
​19: "about:downloads"
​​​22: "about:home"
​​​​​29: "about:memory"
​​33: "about:newtab"
​34: "about:performance"
​​​​​​43: "about:processes"
​​​​59: "about:third-party"
​​​62: "about:webrtc"

We can't do about:sync-log because we don't support dark theme for directory listings - that seems like something we should fix separately.

I think about:newtab is https://searchfox.org/mozilla-central/rev/65d4d3399afa79c8de5a0cc11752d2ba7c31edc1/browser/components/newtab/bin/render-activity-stream-html.js#56-65 + the "prerendered" doc but it'd be good to get that confirmed by the new tab folks.

I'm not 100% sure about about:devtools-toolbox.

Fixing about:credits is a job for the web team (it's a redirect to mozilla.org).

Fixing this involves adding the color-scheme meta tag to about: pages that don't have it yet - listed at the bottom of comment 0. You can use pages that do have it (e.g. https://searchfox.org/mozilla-central/rev/70b32246fce5ca1f53af573a21c1939df58cb969/browser/components/preferences/preferences.xhtml#33 ) as an example.

Mentor: gijskruitbosch+bugs
Severity: -- → S3
Keywords: good-first-bug
Priority: -- → P3

Hi, I would be interesting in taking this bug, if no one else is working on it :)

Assignee: nobody → mpj.5
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(gijskruitbosch+bugs)
Status: NEW → ASSIGNED

I could not find any pages to edit for about:cache-entry, about:devtools-toolbox, about:downloads, about:home or about:newtab - have I missed something?

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Patrick from comment #4)

I could not find any pages to edit for about:cache-entry,

Plot twist, this is in C++! https://searchfox.org/mozilla-central/rev/dc323d0d9a3b722ca8ff0d1b2b752e5bd00dab9b/netwerk/protocol/about/nsAboutCacheEntry.cpp#146

about:devtools-toolbox, about:downloads, about:home or about:newtab - have I missed something?

Let's leave about:devtools-toolbox alone for now... for about:downloads, you want https://searchfox.org/mozilla-central/source/browser/components/downloads/content/contentAreaDownloadsView.xhtml - though tbh I'm not sure we can actually do something that'll work there without converting it to XHTML proper (ie giving it a html tag as root, and a body element - but that might have other implications and is more work) - Emilio, can you comment?

For about:home and about:newtab, I think:

(In reply to :Gijs (he/him) from comment #0)

I think about:newtab is https://searchfox.org/mozilla-central/rev/65d4d3399afa79c8de5a0cc11752d2ba7c31edc1/browser/components/newtab/bin/render-activity-stream-html.js#56-65 + the "prerendered" doc but it'd be good to get that confirmed by the new tab folks.

Meg, can you confirm these are the right places to add meta tags for that should affect about:home/about:newtab ? Thanks!

Flags: needinfo?(mviar)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(emilio)

(In reply to :Gijs (he/him) from comment #5)

Let's leave about:devtools-toolbox alone for now... for about:downloads, you want https://searchfox.org/mozilla-central/source/browser/components/downloads/content/contentAreaDownloadsView.xhtml - though tbh I'm not sure we can actually do something that'll work there without converting it to XHTML proper (ie giving it a html tag as root, and a body element - but that might have other implications and is more work) - Emilio, can you comment?

That would be nice, indeed, but sounds like a separate bug at least. The meta tag is not needed in XUL docs because those are by definition privileged and already behave as-if <meta name=color-scheme content="light dark"> was specified. But specifying it anywhere in the document (so <html:meta .../> should technically work. I don't think it's worth it though.

Flags: needinfo?(emilio)

:Gjis This tag should go in render-activity-stream-html.js as well as /abouthomecache/page.html.template (here). You'll also want to run npm run bundle:html to regenerate the pre-rendeded html in browser/components/newtab/prerendered/ using the npm run bundle:html command.

Flags: needinfo?(mviar)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f2f23757c34f
Added color-scheme meta tag to remaining about: pages r=Gijs,necko-reviewers
Flags: needinfo?(mpj.5)
Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/3b93bac254e8
Added color-scheme meta tag to remaining about: pages r=Gijs,necko-reviewers,robwu
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 97 Branch

The patch landed in nightly and beta is affected.
:mpj.5, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(mpj.5)

Oops.

Flags: needinfo?(mpj.5)
QA Whiteboard: [qa-97b-p2]

Hi, Patrick
Can you please provide some STR in order for me to verify the fix?
Thank you.

Flags: needinfo?(mpj.5)

Doesn't really have STR as it wasn't a bug as such, more just some adjustments to the code. Pinging Gijs in case I'm missing something

Flags: needinfo?(mpj.5) → needinfo?(gijskruitbosch+bugs)

(In reply to Oana Botisan, Desktop Release QA from comment #14)

Hi, Patrick
Can you please provide some STR in order for me to verify the fix?
Thank you.

As Patrick suggested, I don't think this is QA-verifiable, sorry.

Flags: qe-verify-
Flags: needinfo?(oana.botisan)
Flags: needinfo?(gijskruitbosch+bugs)

Seeing there is nothing more I can do here. I will take out the flag for needinfo? on myself.

Flags: needinfo?(oana.botisan)
You need to log in before you can comment on or make changes to this bug.