1691755 - View Source page missing context menu items("Go to Line..." "Wrap Long Lines", "Syntax Highlighting")

Status: RESOLVED FIXED

(Toolkit :: View Source, defect)

Description

[Alice0775 White]

[Tracking Requested - why for this release]: broken view page source feature,

Reproducible always:

Steps to Reproduce:

1. Open any web page
2. Right click on the page, Choose "View Page Source" so that the Page Source will open in a tab
3. Right click on the source page

OR

Steps to Reproduce:

1. Open any web page which has some errors/warnings of js
2. Open "Browser Console"
3. Click on file name in the console so that the file will open in a tab
4. Right click on the source page

Actual Results:
missing context menu items("Go to Line..." "Wrap Long Lines", "Syntax Highlighting")

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=840349cabe2f7e724539ab1b7c50a992f77af0d5&tochange=4ff8babaa7740957fa11b95551c556652c4d0735

Comment 1

[Alice0775 White]

Workaround:
Setting dom.menuitem.enabled to true

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

This seems to be the relevant function: https://searchfox.org/mozilla-central/rev/7067bbd8194f4346ec59d77c33cd88f06763e090/toolkit/actors/ViewSourcePageChild.jsm#507

Comment 3

[:Gijs (he/him)]

Kagami, are you planning to fix this before it hits release? It's tracking 86 release and we're running out of betas.

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

I'm not, but I can try.

Comment 5

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1691755 - Move view-source context menu items to nsContextMenu r=gijs

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8095eaef5dc
Move view-source context menu items to nsContextMenu r=Gijs

Comment 7

[Alexandru Michis [:malexandru]]

Backed out changeset d8095eaef5dc (Bug 1691755) for causing bc failures in browser_viewsourceprefs.js

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

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&fromchange=c1a6871d6ac2b5de092236443bce23a72cc1f34e&searchStr=bc&tochange=d8095eaef5dc892269a253adf71dc4a93ff44820&test_paths=toolkit%2Fcomponents%2Fviewsource%2Ftest%2Fbrowser&selectedTaskRun=Wf9b8w_MT1y0c6drt56vGg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=329815752&repo=autoland&lineNumber=3057

Comment 8

[Kagami Rosylight [:saschanaz] (they/them)]

Oh, so it turns out there actually is a test. I'll fix it next week.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b472dea7eb0c
Move view-source context menu items to nsContextMenu r=Gijs

Comment 10

[Narcis Beleuzu [:NarcisB]]

Backed out for bc failures on browser_viewsourceprefs.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/58ad4dc719e97e3627d2e2542dd9a66bfae03a33
Log link: https://treeherder.mozilla.org/logviewer?job_id=330079072&repo=autoland&lineNumber=1548

Comment 11

[Kagami Rosylight [:saschanaz] (they/them)]

BrowserTestUtils.synthesizeMouseAtCenter is failing 🤔 (Intermittently, because it was successful in https://treeherder.mozilla.org/jobs?repo=try&revision=24120059e9f0b26e63749446e128d3e730f19f73)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/008bb6521b86
Move view-source context menu items to nsContextMenu r=Gijs

Comment 13

[Kagami Rosylight [:saschanaz] (they/them)]

Are we okay with the wontfix for 86? My understanding is that it's not?

Comment 14

[:Gijs (he/him)]

(In reply to Kagami :saschanaz from comment #13)

Are we okay with the wontfix for 86? My understanding is that it's not?

Well, it's not great, but we built 86.0 release candidates already, so I imagine it got marked wontfix because we're not planning on additional rc builds before we release next week. You can see the detailed release calendar from the link on https://wiki.mozilla.org/Release_Management/Calendar . Ryan, do you know what the state is there?

I only realized this was broken last week (and, apparently, so did Alice), or this would have been flagged sooner, but it may not be avoidable to ship the regression at this point, unless we think this on its own qualifies for an rc respin, or unless something else comes up that trips an rc build with which we can take this as a ride-along (and we're confident about the stability of the patch).

Comment 15

[Ryan VanderMeulen [:RyanVM]]

This was discussed during regression triage today and the decision to wontfix was based on where we were in the release cycle (as you noted, we've already built the 86 RC and don't have any current plans to respin), the fact that this wasn't a new regression in 86, and that the patch had already landed and been backed out multiple times. If you feel strongly that the decision should be reconsidered, you can discuss with Pascal.

Comment 16

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/008bb6521b86