1857999 - Stop using xml-stylesheet processing instructions for styling non-test xhtml documents

Status: RESOLVED FIXED

(Firefox :: General, task)

Description

[:Gijs (he/him)]

These are getting misformatted by prettier v3, and it's about time we stop using them anyway. link elements are more widely recognized by tooling, and have more features than XML PIs.

(For instance, using "normal" link elements with their media attributes would provide a neat way to e.g. include platform-specific styles in a way that doesn't require ifdefs or media queries combined with @import in a single shared sheet.)

It's worth noting that not all our consumers live in markup, cf. https://searchfox.org/mozilla-central/rev/d265ec4f5fdcb5ad0c06636c5ac010ef4ff7eb89/toolkit/modules/SubDialog.sys.mjs#111-121 .

Comment 1

[:Gijs (he/him)]

Logan, I don't suppose you'd be interested in picking this up given how it relates to bug 1857834? ("No, I don't have time" or w/e is fine, no pressure. :-) )

Comment 2

[Logan Rosen [:Logan]]

I'm not sure I know enough about the inner workings of these XHTML documents to do this justice, unfortunately.

Comment 4

[Dave Townsend [:mossop]]

Attachment: Bug 1857999: SubDialog should insert stylesheets as link elements. r=gijs!

Attempt to find other link elements in the document and insert the injected
stylesheets after those. Strictly speaking we could probably just insert them
at the end of the document but this at least keeps them together with other
link elements to aid in inspecting. Falls back to using the XML processing
instruction in hte unlikely case that no other link elements are present.

Comment 5

[Dave Townsend [:mossop]]

Attachment: Bug 1857999: Add html namespace declarations for files that are missing it. r=gijs!

The automated scripts requires an already present namespace prefix for the
xhtml namespace so just manually add those for documents that don't already have
one.

Depends on D190679

Comment 6

[Dave Townsend [:mossop]]

Attachment: Bug 1857999: Move XML stylesheet processing instructions to link elements. r=gijs!

The result of running the automated script available at https://github.com/Mossop/depi

Depends on D190680

Comment 7

[Dave Townsend [:mossop]]

Attachment: Bug 1857999: Manually fix some spacing and comments. r=gijs!

Manually re-apply comments and spacing that existed prior to the automated
changes.

Depends on D190681

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c25b3a0b864c
SubDialog should insert stylesheets as link elements. r=Gijs,settings-reviewers

Comment 9

[Dave Townsend [:mossop]]

Attachment: Bug 1857999: Devtools should always inject additional stylesheets as link elements. r=ochameau!

Improves the logic to always insert stylesheets as link elements regardless of
whether it is into a html document or not. In order for the ordering to be
correct the insertion must occur after existing link elements in the document
have been parsed so the theme-switching.js script must always be loaded after
link elements. The storage client appears to be the only existing document where
that wasn't already the case.

Comment 10

[Cristian Tuns]

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd5a10f9cfff
Devtools should always inject additional stylesheets as link elements. r=ochameau,devtools-reviewers
https://hg.mozilla.org/integration/autoland/rev/b8a03d31ecea
Add html namespace declarations for files that are missing it. r=Gijs,webdriver-reviewers,devtools-reviewers
https://hg.mozilla.org/integration/autoland/rev/0d0c1d1e80e7
Move XML stylesheet processing instructions to link elements. r=Gijs,webdriver-reviewers,settings-reviewers,application-update-reviewers,devtools-reviewers,places-reviewers,Standard8,whimboo,bytesized,ochameau
https://hg.mozilla.org/integration/autoland/rev/75f5e6e3d917
Manually fix some spacing and comments. r=Gijs,settings-reviewers,places-reviewers,Standard8

Comment 12

[Sandor Molnar[:smolnar]]

Backed out for causing for causing dt failures on browser_theme_switching.js & Mn failures at test_element_state_chrome.py

Backout link: https://hg.mozilla.org/integration/autoland/rev/ee6b5dc47ec451962b3c7c27e1480d1a94177c63

Push with failures

Failure log dt -> TEST-UNEXPECTED-FAIL | devtools/client/shared/test/browser_theme_switching.js | There is a stylesheet for chrome://devtools/skin/light-theme.css

Failure log Mn ->TEST-UNEXPECTED-FAIL | testing/marionette/harness/marionette_harness/tests/unit/test_element_state_chrome.py TestElementState.test_enabled | AssertionError: True is not false

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5dd474fcfe1f
Devtools should always inject additional stylesheets as link elements. r=ochameau,devtools-reviewers
https://hg.mozilla.org/integration/autoland/rev/86eaaa793b27
Add html namespace declarations for files that are missing it. r=Gijs,webdriver-reviewers,devtools-reviewers
https://hg.mozilla.org/integration/autoland/rev/fcfb26498574
Move XML stylesheet processing instructions to link elements. r=Gijs,webdriver-reviewers,settings-reviewers,application-update-reviewers,devtools-reviewers,places-reviewers,Standard8,whimboo,bytesized,ochameau
https://hg.mozilla.org/integration/autoland/rev/0cf1d9d7091d
Manually fix some spacing and comments. r=Gijs,settings-reviewers,places-reviewers,Standard8

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/5dd474fcfe1f
https://hg.mozilla.org/mozilla-central/rev/86eaaa793b27
https://hg.mozilla.org/mozilla-central/rev/fcfb26498574
https://hg.mozilla.org/mozilla-central/rev/0cf1d9d7091d