Closed Bug 1772906 Opened 2 months ago Closed 2 months ago

View source in external editor stopped working

Categories

(Toolkit :: View Source, defect)

Firefox 102
defect
Points:
3

Tracking

()

RESOLVED FIXED
103 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox-esr102 --- fixed
firefox101 --- unaffected
firefox102 + fixed
firefox103 --- fixed

People

(Reporter: bugzilla, Assigned: enndeakin)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [fidefe-Quality-Foundation])

Attachments

(1 file)

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

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

This is a regression.

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.

Component: Untriaged → DOM: Editor
Product: Firefox → Core

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

Status: UNCONFIRMED → NEW
Component: DOM: Editor → View Source
Ever confirmed: true
Product: Core → Toolkit
Regressed by: 1746052

[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).

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

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

Flags: needinfo?(enndeakin)
Has Regression Range: --- → yes

(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.

Assignee: nobody → enndeakin
Attachment #9280083 - Attachment description: WIP: Bug 1772906, switch to use validateFileNameForSaving to verify the filename when opening view source in an external editor, r=mak → Bug 1772906, switch to use validateFileNameForSaving to verify the filename when opening view source in an external editor, r=mak
Status: NEW → ASSIGNED
Flags: needinfo?(enndeakin)
Points: --- → 3
Whiteboard: [fidefe-Quality-Foundation]
Duplicate of this bug: 1773587

(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...

Pushed by neil@mozilla.com:
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

Backed out for causing failures on browser_validatefilename.js

[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
Flags: needinfo?(enndeakin)

Any luck?

Pushed by neil@mozilla.com:
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

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"
Pushed by neil@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 103 Branch
Flags: needinfo?(enndeakin)

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.

Flags: needinfo?(enndeakin)

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
Flags: needinfo?(enndeakin)
Attachment #9280083 - Flags: approval-mozilla-beta?

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.

Attachment #9280083 - Flags: approval-mozilla-beta? → approval-mozilla-release?

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.

Attachment #9280083 - Flags: approval-mozilla-release?
Attachment #9280083 - Flags: approval-mozilla-release+
Attachment #9280083 - Flags: approval-mozilla-esr102+
You need to log in before you can comment on or make changes to this bug.