Activity
Replying to cypherpunks:
Does this also need to be done for the changelog about page popup or is it the same page as about:tor?
It is a separate page (about:tbupdate) but it already runs with only content privileges.
Replying to cypherpunks:
Actual for e10s.
Do you mean that with e10s enabled, we must avoid requiring chrome privileges? I do not think that is true, so maybe you mean something else?
Here is a patch for review: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug18913-01&id=984af558af58bb8715af72c4811acc7fc4253bc1 This change fixes #21948 (moved) and #22525 (moved) as well, so it would be great to include it in a Tor Browser release soon. While the patch is somewhat large, that is mainly because we had to move a lot of code out of torbutton.js into the new aboutTor-content.js content script (so it can run in the content process where the about:tor DOM is accessible).
Trac:
Keywords: TorBrowserTeam201706 deleted, TorBrowserTeam201706R added
Status: new to needs_review
Replying to mcs:
Here is a patch for review: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug18913-01&id=984af558af58bb8715af72c4811acc7fc4253bc1 This change fixes #21948 (moved) and #22525 (moved) as well, so it would be great to include it in a Tor Browser release soon. While the patch is somewhat large, that is mainly because we had to move a lot of code out of torbutton.js into the new aboutTor-content.js content script (so it can run in the content process where the about:tor DOM is accessible).
Looks good to me, thanks! Just some nits:
+ // process that is only available here (in the chrome process). It is sent + // sent to the content process when an about:tor window is opened and injust one "sent"
+ kAboutTorMessages: [ "AboutTor:ChromeData", "AboutTor:ToolbarData" ], + + get isAboutTor() { + return content.document.documentURI.toLowerCase() == "about:tor";Indentation
"the Tor Button item's x coordinate" -> "the x coordinate of Torbutton's toolbar item"
"torbutton toolbar item" -> "Torbutton toolbar item"
Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201706R deleted, TorBrowserTeam201706 addedReplying to gk:
Looks good to me, thanks! Just some nits: ...
Thanks for your review! Here is a revised patch that fixes the things you found: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug18913-02
Trac:
Status: needs_revision to needs_review
Keywords: TorBrowserTeam201706 deleted, TorBrowserTeam201706R addedSorry, I should have given more context. You have now
+var AboutTorListener = { + kAboutTorMessages: [ "AboutTor:ChromeData", "AboutTor:ToolbarData" ], + + get isAboutTor() { + return content.document.documentURI.toLowerCase() == "about:tor"; + },but I'd like to have it like
+var AboutTorListener = { + kAboutTorMessages: [ "AboutTor:ChromeData", "AboutTor:ToolbarData" ], + + get isAboutTor() { + return content.document.documentURI.toLowerCase() == "about:tor"; + },Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201706R deleted, TorBrowserTeam201706 addedI am sorry we botched the indentation fix, and I am sorry that we then forgot to revise the patch for the past two days. Do you have a git tool to spot those kind of errors, or just "eagle eyes?
In any case, here is a revised patch: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug18913-03
Trac:
Status: needs_revision to needs_review
Keywords: TorBrowserTeam201706 deleted, TorBrowserTeam201706R added