Closed Bug 1116880 Opened 9 years ago Closed 9 years ago

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

Categories

(Toolkit :: View Source, defect)

37 Branch
x86_64
Windows 8.1
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox35 --- unaffected
firefox36 + fixed
firefox37 + fixed

People

(Reporter: streetwolf52, Assigned: ehsan.akhgari)

References

(Depends on 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file)

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.
Component: Untriaged → View Source
Product: Firefox → Toolkit
Blocks: e10s
Summary: [E10s] Pref 'view_source.editor.path' doesn't work with e10s → [e10s] Pref 'view_source.editor.path' doesn't work with e10s
tracking-e10s: --- → ?
Hardware: x86 → x86_64
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
[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
Summary: [e10s] Pref 'view_source.editor.path' doesn't work with e10s → [e10s] [non-e10s] Pref 'view_source.editor.path' doesn't work
tracking-e10s: ? → ---
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?
(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
err
s/referrer uri/referrer policy/
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)
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: 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
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Ehsan, this seems to affect 36, could you fill the uplift request to aurora? Thanks
Flags: needinfo?(ehsan)
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.