Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29352667: Issue 4398 - Fix "view source in a separate window" command (Closed)

Sept. 12, 2016, 12:36 p.m. by Wladimir Palant
Sept. 13, 2016, 4:02 p.m.
Base URL:


Issue 4398 - Fix "view source in a separate window" command

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -6 lines) Patch
M lib/aardvark.js View 2 chunks +28 lines, -6 lines 3 comments Download


Total messages: 4
Wladimir Palant
Sept. 12, 2016, 12:36 p.m. (2016-09-12 12:36:21 UTC) #1
(Not familiar with this code but will try my best to review.) https://codereview.adblockplus.org/29352667/diff/29352668/lib/aardvark.js File lib/aardvark.js ...
Sept. 13, 2016, 9:49 a.m. (2016-09-13 09:49:26 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29352667/diff/29352668/lib/aardvark.js File lib/aardvark.js (right): https://codereview.adblockplus.org/29352667/diff/29352668/lib/aardvark.js#newcode709 lib/aardvark.js:709: URI: "view-source:data:text/html;charset=utf-8," + encodeURIComponent(elem.outerHTML), On 2016/09/13 09:49:26, kzar wrote: ...
Sept. 13, 2016, 10:32 a.m. (2016-09-13 10:32:08 UTC) #3
Sept. 13, 2016, 12:06 p.m. (2016-09-13 12:06:50 UTC) #4

File lib/aardvark.js (right):

lib/aardvark.js:709: URI: "view-source:data:text/html;charset=utf-8," +
On 2016/09/13 10:32:08, Wladimir Palant wrote:
> On 2016/09/13 09:49:26, kzar wrote:
> > How come URI, drawSelection and baseURI are added here? I can't see those in
> the
> > old code for earlier versions of Gecko.
> You have to look at the changes introduced in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1134585, particularly the new
> getSelection function. It's a E10S fix - the View Source dialog is running in
> the chrome process, it cannot access the selection (living in the content
> process) directly. So rather than accepting a selection it requires the URI to
> be displayed - originally that was being done by the dialog code. Note that
> there is also some trickery to mark the code to be selected, I skipped that
> set drawSelection to false as it doesn't add any real value.
> It's rather unfortunate that we are relying on Firefox implementation details
> here. Then again, it's the first time this particular code breaks, the calling
> convention for this dialog doesn't change too often.
> Thinking a bit more about it, we could probably select this particular node in
> the document and trigger the "View Selection Source" command from the context
> menu. Not sure whether this would be better however, it would then rely on
> application details whereas the current code is only relying on the toolkit
> which is shared among all applications.

That makes sense, thanks for explaining. (I tried having a look through the
related Firefox change you linked but obviously couldn't quickly grok it all.
I'll take your word for it!)

Powered by Google App Engine
This is Rietveld