Line wrapping and syntax highlighting do not work on view-source pages

https://blog.torproject.org/comment/241854#comment-241854 pointed out that view-source pages are broken wrt line wrapping. Additionally, it turns our that syntax highlighting is affected as well. This is a regression caused by #8725 (moved).

added TorBrowserTeam201706R component::applications/tor browser owner::tbb-team priority::medium resolution::fixed severity::normal status::closed tbb-6.5-issues tbb-regression type::defect labels

Hm. So, this got introduced with #8725 (moved). I have a branch (bug_22457 (https://gitweb.torproject.org/user/gk/torbutton.git/commit/?h=bug_22457&id=cb12ea9247fae28bb10c4ad3fb4e751147adcc76) ) that fixes the problem by adding viewsource.css to the whitelist. However, the CSS file for Linux is significantly different from that for Windows/macOS allowing probably detection of Linux/non-Linux platforms.

While we are not actively protecting against platform detection it seems to me we should avoid adding additional means if we can. Thus, I am proposing to WONTFIX this issue especially as the vast majority of users should be unaffected by this bug due to e10s being enabled by default.

Trac:
Cc: N/A to yawning, mcs, brade, arthuredelstein

Trac:
Keywords: N/A deleted, tbb-6.5-issues, tbb-regression added

Replying to gk:

While we are not actively protecting against platform detection it seems to me we should avoid adding additional means if we can. Thus, I am proposing to WONTFIX this issue especially as the vast majority of users should be unaffected by this bug due to e10s being enabled by default.

I would be ok with it being whitelisted or not (I think platform detection is totally a lost cause, and will be forever). Perhaps it should be whitelisted only if e10s is disabled.

nb: The "better" solution to the resource/chrome URI situation (where all requests originating from the local browser are whitelisted) would fix this.

Trac:
Keywords: N/A deleted, TorBrowserTeam201705 added

I think I have a better fix for that: view-source should not be available anymore in esr52, so we can allow it unconditionally I think.

https://bugzilla.mozilla.org/show_bug.cgi?id=1172165 is the bug I had in mind.

Replying to gk:

I think I have a better fix for that: view-source should not be available anymore in esr52, so we can allow it unconditionally I think.

On second thought that might not help much as the blocked CSS is the problem.

You can experiment with https://bugzilla.mozilla.org/show_bug.cgi?id=863246, because they're going to land it.

Resolved #22440 (moved) as duplicate.

Trac:
Cc: yawning, mcs, brade, arthuredelstein to yawning, mcs, brade, arthuredelstein, gapegas7uftp

Replying to gk:

Replying to gk:

I think I have a better fix for that: view-source should not be available anymore in esr52, so we can allow it unconditionally I think.

On second thought that might not help much as the blocked CSS is the problem.

But we can work around that using aRequestOrigin. See bug_22457_v2 () for a possible fix that avoids the plaform fingerprinting. Note: For testing purposes you need either disabled e10s or apply the patch on top of the one in #22459 (moved).

Trac:
Summary: Line wrapping and syntax highlighting do not work in non-e10s-mode on view-source pages to Line wrapping and syntax highlighting do not work on view-source pages
Description: While the switch to multiprocess mode solves the linewrapping issue (#22440 (moved)) on view-source pages for the vast majority of our users, those that don't have that mode activated are still affected.

to

https://blog.torproject.org/comment/241854#comment-241854 pointed out that view-source pages are broken wrt line wrapping. Additionally, it turns our that syntax highlighting is affected as well. This is a regression caused by #8725 (moved).
Status: new to needs_review
Keywords: TorBrowserTeam201705 deleted, TorBrowserTeam201705R added

Trac:
Keywords: TorBrowserTeam201705R deleted, TorBrowserTeam201706R added

Replying to gk:

Replying to gk:

Replying to gk:

I think I have a better fix for that: view-source should not be available anymore in esr52, so we can allow it unconditionally I think.

On second thought that might not help much as the blocked CSS is the problem.

But we can work around that using aRequestOrigin. See bug_22457_v2 () for a possible fix that avoids the plaform fingerprinting. Note: For testing purposes you need either disabled e10s or apply the patch on top of the one in #22459 (moved).

I couldn't find the bug_22457_v2 patch. Does it need to be pushed?

I agree with Yawning that hiding differences between platforms is a hopeless cause, so I would also be happy with whitelisting the appropriate css files.

Replying to arthuredelstein:

Replying to gk:

Replying to gk:

Replying to gk:

I think I have a better fix for that: view-source should not be available anymore in esr52, so we can allow it unconditionally I think.

On second thought that might not help much as the blocked CSS is the problem.

But we can work around that using aRequestOrigin. See bug_22457_v2 () for a possible fix that avoids the plaform fingerprinting. Note: For testing purposes you need either disabled e10s or apply the patch on top of the one in #22459 (moved).

I couldn't find the bug_22457_v2 patch. Does it need to be pushed?

It did indeed: https://gitweb.torproject.org/user/gk/torbutton.git/commit/?h=bug_22457_v2&id=7d65c8297fe862906bcb3d7c9c13e5c5b628943a

r=brade, r=mcs The code changes look fine and the patch seems to fix the problem (we did a quick test on OSX).

Thanks. Cherry-picked onto master (commit 137c0527b1d152c5999db53894badc54ab9e34c9).

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

closed

moved to tpo/applications/tor-browser#22457 (closed)