[e10s] [non-e10s] Pref 'view_source.editor.path' doesn't work

RESOLVED FIXED in Firefox 36

Status

()

Toolkit
View Source
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: streetwolf, Assigned: Ehsan)

Tracking

(Depends on: 1 bug, {regression})

37 Branch
mozilla37
x86_64
Windows 8.1
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 unaffected, firefox36+ fixed, firefox37+ fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20141231103019

Steps to reproduce:

1. Make sure e10s are enabled.

2. Set pref 'view_source.editor.path' to a text editor like Notepad.

3. Select 'View Page Source' from the context menu


Actual results:

The default Fx editor is invoked not the one I specified in the pref.

My text editor get's invoked with e10s disabled.


Expected results:

My text editor should have been invoked with e10s enabled.
(Reporter)

Updated

3 years ago
Component: Untriaged → View Source
Product: Firefox → Toolkit
(Reporter)

Updated

3 years ago
Blocks: 516752
Summary: [E10s] Pref 'view_source.editor.path' doesn't work with e10s → [e10s] Pref 'view_source.editor.path' doesn't work with e10s
(Reporter)

Updated

3 years ago
tracking-e10s: --- → ?
(Reporter)

Updated

3 years ago
Hardware: x86 → x86_64

Comment 1

3 years ago
user_pref("view_source.editor.external", true);
user_pref("view_source.editor.path", "C:\\windows\\system32\\notepad.exe");


An error is shown in Browser Console:

[Exception... "Not enough arguments [nsIWebBrowserPersist.savePrivacyAwareURI]"  nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)"  location: "JS frame :: chrome://global/content/viewSourceUtils.js :: gViewSourceUtils.openInExternalEditor :: line 133"  data: no] viewSourceUtils.js:162:0
Blocks: 704320
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(sstamm)
Keywords: regression

Comment 2

3 years ago
[Tracking Requested - why for this release]: affected non-e10s window on Nightly37.0a1 and Aurora36.0a2

Steps to reproduce:
0. Make sure External source viewer setup.
1. Open non-e10s window of Nightly37.0a1 or Aurora36.0a2
2. Open "Browser Console" (Ctrl + Shift + J)
3. Open any page which has error/warning
4. Click a link of source file name in the "Browser Console"

Actual results:
Source opened in built-in source viewer

Expected Results:
Source should open in external source viewer
status-firefox35: --- → unaffected
status-firefox36: --- → affected
status-firefox37: --- → affected
tracking-firefox36: --- → ?
tracking-firefox37: --- → ?
Summary: [e10s] Pref 'view_source.editor.path' doesn't work with e10s → [e10s] [non-e10s] Pref 'view_source.editor.path' doesn't work

Updated

3 years ago
tracking-e10s: ? → ---
(Reporter)

Comment 3

3 years ago
I'm a little confused Alice.  With non-e10s, viewing the Page Source via the context menu brings up my external viewer.  Following the STR in comment 2 opens up the built in viewer. Could this be two similar problems?

Comment 4

3 years ago
(In reply to Gary [:streetwolf] from comment #3)
> I'm a little confused Alice.  With non-e10s, viewing the Page Source via the
> context menu brings up my external viewer.  Following the STR in comment 2
> opens up the built in viewer. Could this be two similar problems?

Yes, they are exactly same problem due to missing argument which is referrer uri.
Regressed by Bug 704320

Comment 5

3 years ago
err
s/referrer uri/referrer policy/

Updated

3 years ago
Depends on: 1117109
Yep.  An automated test would be nice to catch regressions like this in the future.  Probably just need to pass another argument to nsIWebBrowserPersist.saveURI() (it can be null).
Flags: needinfo?(sstamm)
(Assignee)

Comment 7

3 years ago
Testing this would be rather tricky since it will effectively launch another process.  We have had issues with other apps running on the same desktop when our tests are running.
(Assignee)

Comment 8

3 years ago
Created attachment 8543368 [details] [diff] [review]
Pass in the referrer policy to the savePrivacyAwareURI() call in openInExternalEditor()
Attachment #8543368 - Flags: review?(sstamm)
(Assignee)

Updated

3 years ago
Assignee: nobody → ehsan
Comment on attachment 8543368 [details] [diff] [review]
Pass in the referrer policy to the savePrivacyAwareURI() call in openInExternalEditor()

Review of attachment 8543368 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/viewsource/content/viewSourceUtils.js
@@ +130,5 @@
>            // the default setting is to not decode. we need to decode.
>            webBrowserPersist.persistFlags = this.mnsIWebBrowserPersist.PERSIST_FLAGS_REPLACE_EXISTING_FILES;
>            webBrowserPersist.progressListener = this.viewSourceProgressListener;
> +          let referrerPolicy = Components.interfaces.nsIHttpChannel.REFERRER_POLICY_NO_REFERRER_WHEN_DOWNGRADE;
> +          webBrowserPersist.savePrivacyAwareURI(uri, null, null, referrerPolicy, null, null, file, fromPrivateWindow);

You could also use REFERRER_POLICY_NO_REFERRER or null (doesn't really matter since there's no referrer URI passed in).
Attachment #8543368 - Flags: review?(sstamm) → review+
https://hg.mozilla.org/mozilla-central/rev/6d124760473c
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Ehsan, this seems to affect 36, could you fill the uplift request to aurora? Thanks
status-firefox37: affected → fixed
tracking-firefox36: ? → +
tracking-firefox37: ? → +
Flags: needinfo?(ehsan)
(Assignee)

Comment 12

3 years ago
Comment on attachment 8543368 [details] [diff] [review]
Pass in the referrer policy to the savePrivacyAwareURI() call in openInExternalEditor()

Approval Request Comment
[Feature/regressing bug #]: Bug 704320
[User impact if declined]: Comment 0
[Describe test coverage new/current, TBPL]: Tested locally
[Risks and why]: Low risk
[String/UUID change made/needed]: none
Flags: needinfo?(ehsan)
Attachment #8543368 - Flags: approval-mozilla-aurora?
Attachment #8543368 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.