Closed Bug 1534407 Opened 9 months ago Closed 7 months ago

Enable browser.xhtml by default

Categories

(Firefox :: General, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 69
Tracking Status
firefox69 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file)

After everything is green, we'll need to:

  1. Provide a default value as true for the MOZ_BROWSER_XHTML define (presumably still allowing it to be explicitly set to false for testing purposes): https://searchfox.org/mozilla-central/rev/8ff2cd0a27e3764d9540abdc5a66b2fb1e4e9644/browser/base/moz.build#64
  2. Write a XULStore migration from browser.xul to browser.xhtml: https://searchfox.org/mozilla-central/rev/8ff2cd0a27e3764d9540abdc5a66b2fb1e4e9644/browser/components/BrowserGlue.jsm#2266. That'd be something like the xulStore.getValue(BROWSER_DOCURL, ...) calls already in there, but we'll want to hardcode "chrome://browser/content/browser.xul" as the source and "chrome://browser/content/browser.xhtml" as the target of the copying. I guess we'll want to use getAttributeEnumerator and loop over browser.xul in the store and write them into browser.xhtml. Maybe we should expose a XULStore function to wrap up this "document migration" case since we could want it for future xul->xhtml migrations.

Do we intend to do something like this prior to being convinced we can make it ride that train? I'd be pretty worried about making nightly quite that different from what we end up shipping...

Flags: needinfo?(bgrinstead)

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

Do we intend to do something like this prior to being convinced we can make it ride that train? I'd be pretty worried about making nightly quite that different from what we end up shipping...

Agreed, and this would happen only after we are confident it will ship. Right now we are using try and talos as the goalposts for this, so if they are both green after the merge I will propose we do it. Do you know of other criteria we should be tracking or sign-off we should be seeking before we land?

Flags: needinfo?(bgrinstead) → needinfo?(gijskruitbosch+bugs)

I haven't been following this in detail, so I'm not really the right person to ask. There's some open deps on bug 1533881 that I guess we should ensure we feel are either not blockers to ship or we're very confident are going to be resolved post-merge? The a11y angle seems like a blocker; we can't ship a Firefox that a good portion of our users can't use anymore...

Flags: needinfo?(gijskruitbosch+bugs)

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

I haven't been following this in detail, so I'm not really the right person to ask. There's some open deps on bug 1533881 that I guess we should ensure we feel are either not blockers to ship or we're very confident are going to be resolved post-merge? The a11y angle seems like a blocker; we can't ship a Firefox that a good portion of our users can't use anymore...

Of course. I thought that was obvious - sorry that it wasn't clear. We aren't planning to land this before we get the sign off from accessibility team in Bug 1488918, and have resolutions for other important blockers. Let me reorganize this - changing this from blocking the meta to depending on the meta.

No longer blocks: 1533881
Depends on: 1533881

Did a talos push and seeing a few medium confidence ts_paint regressions along with one strong on startup_about_home_paint opt e10s stylo (although that one is QR and I don't know how stable those results are now): https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=015844ab39ebe7f836c6647ee9392592a49cedb4&newProject=try&newRevision=590b11ff486fd121052604db2d5f6e15eccc1a6e&framework=1.

So there may still be a bit more to do here on perf, but it's way closer than it was pre Bug 1527977.

Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Duplicate of this bug: 1483277
Blocks: 1551334
Blocks: 1492582

Bug 1552058 is spinning out some of the patch here for landing separately, so I'll rebase around that once it lands.

Depends on: 1552058
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ad38bcbe7879
Enable browser.xhtml by default r=mossop
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
Blocks: 1553188
Regressions: 1553387
You need to log in before you can comment on or make changes to this bug.