1772906 - View source in external editor stopped working

Status: RESOLVED FIXED

(Toolkit :: View Source, defect)

Description

[Alex]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Firefox/102.0

Steps to reproduce:

Verify that view_source.editor.external = true
Verify that view_source.editor.path = C:\Program Files\Notepad++\notepad++.exe (or the path of the text editor of your choice)
Navigate to any web page
Hit CTRL-U

Actual results:

The source opened in the internal source viewer (in a browser tab).

The console shows:

this._caUtils.getDefaultExtension is not a function viewSourceUtils.js:413
    getTemporaryFile chrome://global/content/viewSourceUtils.js:413
    openInExternalEditor chrome://global/content/viewSourceUtils.js:265
    openInExternalEditor chrome://global/content/viewSourceUtils.js:216
    BrowserViewSourceOfDocument chrome://browser/content/browser.js:3131
    BrowserViewSource chrome://browser/content/browser.js:3207
    oncommand chrome://browser/content/browser.xhtml:1

Comment 1

[Alex]

Expected results:
Source opens in external editor, like in previous versions.

This is a regression.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Editor' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

contentAreaUtils.getDefaultExtension was removed by bug 1746052. Clearly there's test-coverage missing there.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

[Tracking Requested - why for this release]: Seems this hasn't hit release yet, we should consider trying to fix this regression before it hits release if it's not too hard (though then again it's a non-default configuration so perhaps it's not super-high-priority).

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1746052

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

:enndeakin, since you are the author of the regressor, bug 1746052, could you take a look?
For more information, please visit auto_nag documentation.

Comment 7

[Alex]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

(though then again it's a non-default configuration so perhaps it's not super-high-priority).

I cannot think of a single developer that would prefer the built-in editor to their external editor of choice, and that also holds for the vast majority of non-developers that have other reasons to frequently access HTML source. I would claim that the reason it is non-default is because everybody has their own preferred editor.

Comment 8

[Neil Deakin]

Attachment: Bug 1772906, switch to use validateFileNameForSaving to verify the filename when opening view source in an external editor, r=mak

Comment 10

[:Gijs (he/him)]

(In reply to Alex from comment #7)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

(though then again it's a non-default configuration so perhaps it's not super-high-priority).

I cannot think of a single developer that would prefer the built-in editor to their external editor of choice, and that also holds for the vast majority of non-developers that have other reasons to frequently access HTML source. I would claim that the reason it is non-default is because everybody has their own preferred editor.

On Firefox at least, I just view source in a tab, which is the default. I don't tend to need or want an actual editor for http-provided source from a site I don't control. I can't meaningfully edit anything! Given these are all about:config prefs that aren't exposed anywhere, I doubt changing it is as common as you suppose. Either way, without telemetry (and there doesn't appear to be any) we can't tell which of us is right in our suppositions about what other people do and do not do...

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9756645733e9
switch to use validateFileNameForSaving to verify the filename when opening view source in an external editor, r=mak

Comment 12

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on browser_validatefilename.js

• backout: https://hg.mozilla.org/integration/autoland/rev/6286f11287837e16570e3ea7150f1c90a0fe4061
• push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=9756645733e93b9d4273e3b8566438ea114c3181&group_state=expanded
• failure log: https://treeherder.mozilla.org/logviewer?job_id=380898693&repo=autoland&lineNumber=29709

[task 2022-06-10T13:48:58.526Z] 13:48:58     INFO - TEST-PASS | toolkit/components/viewsource/test/browser/browser_validatefilename.js | filename title for simple - 
[task 2022-06-10T13:48:58.526Z] 13:48:58     INFO - Buffered messages finished
[task 2022-06-10T13:48:58.528Z] 13:48:58     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/viewsource/test/browser/browser_validatefilename.js | filename title for samplefile - Got "samplefile.txt", expected "samplefile.txt.html"
[task 2022-06-10T13:48:58.528Z] 13:48:58     INFO - Stack trace:
[task 2022-06-10T13:48:58.528Z] 13:48:58     INFO - chrome://mochikit/content/browser-test.js:test_is:1429
[task 2022-06-10T13:48:58.528Z] 13:48:58     INFO - chrome://mochitests/content/browser/toolkit/components/viewsource/test/browser/browser_validatefilename.js:null:69
[task 2022-06-10T13:48:58.529Z] 13:48:58     INFO - Leaving test bound 
[task 2022-06-10T13:48:58.529Z] 13:48:58     INFO - GECKO(12541) | MEMORY STAT | vsize 11515MB | residentFast 467MB | heapAllocated 214MB
[task 2022-06-10T13:48:58.530Z] 13:48:58     INFO - TEST-OK | toolkit/components/viewsource/test/browser/browser_validatefilename.js | took 812ms

Comment 13

[Alex]

Any luck?

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de9ad4f5b1ea
switch to use validateFileNameForSaving to verify the filename when opening view source in an external editor, r=mak

Comment 15

[Cristian Tuns]

Backed out for causing mochitest failures in browser_validatefilename.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | toolkit/components/viewsource/test/browser/browser_validatefilename.js | filename title for index xhtml - Got "index.xhtml", expected "index.xht"
  TEST-UNEXPECTED-FAIL | toolkit/components/viewsource/test/browser/browser_validatefilename.js | filename title for Hello There xhtml - Got "Hello There.xhtml", expected "Hello There.xht"

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bdd0e402bb14
switch to use validateFileNameForSaving to verify the filename when opening view source in an external editor, r=mak

Comment 17

[Atila Butkovits]

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

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:enndeakin, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox102 to wontfix.

For more information, please visit auto_nag documentation.

Comment 19

[Neil Deakin]

Comment on attachment 9280083 [details]
Bug 1772906, switch to use validateFileNameForSaving to verify the filename when opening view source in an external editor, r=mak

Beta/Release Uplift Approval Request

• User impact if declined: Users who have set the custom setting view_source.editor.external won't be able to view a page's source in an external editor, instead just failing with an error to the console.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): No risk as the code is only executed when the non-default setting is changed in about:config and it currently fails completely without this patch.
• String changes made/needed: None
• Is Android affected?: No

Comment 20

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9280083 [details]
Bug 1772906, switch to use validateFileNameForSaving to verify the filename when opening view source in an external editor, r=mak

We are in RC week and the merge is done. Morphing this into a release build uplift request.

Comment 21

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9280083 [details]
Bug 1772906, switch to use validateFileNameForSaving to verify the filename when opening view source in an external editor, r=mak

Taking in our 102.0 build and 102.0esr build as this is low risk, is a 102 regression and has tests, thanks.

Comment 22

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-release/rev/8bad0791002a

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr102/rev/574f0632c8d8