5.34 KB, patch
|Details | Diff | Splinter Review|
59 bytes, text/x-review-board-request
Please see the original Tor ticket: https://trac.torproject.org/21830 Currently we are using the following patch, https://torpat.ch/21830 but in Firefox it would probably be better to use Private Browsing state rather than the "browser.privatebrowsing.autostart" pref.
Component: Developer Tools: Console → Widget
Product: Firefox → Core
Here's an approach that simply avoids writing to /tmp if we aren't sure we're in a private browsing context.
Attachment #8950121 - Flags: review?(josh)
Comment on attachment 8950121 [details] [diff] [review] 0001-Bug-1433030-Stop-nsTransferable-writing-to-tmp-in-PB.patch Review of attachment 8950121 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/tests/test_bug1123480.xul @@ +37,5 @@ > const gClipboardHelper = Components.classes["@mozilla.org/widget/clipboardhelper;1"].getService(Components.interfaces.nsIClipboardHelper); > gClipboardHelper.copyString(Ipsum); > > + // Undefined private browsing mode should cache large selections to disk > + ok(!clipboardFile.exists(), "correctly saved memory by caching to disk"); The comment and assertion message don't match what we're testing.
Attachment #8950121 - Flags: review?(josh) → review+
(In reply to Josh Matthews [:jdm] from comment #3) > Comment on attachment 8950121 [details] [diff] [review] > 0001-Bug-1433030-Stop-nsTransferable-writing-to-tmp-in-PB.patch > [snip] > The comment and assertion message don't match what we're testing. Thanks for the review. Fixed the comment and assertion message.
Attachment #8950121 - Attachment is obsolete: true
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9e19b58d6494 Stop nsTransferable writing to /tmp in PB windows r=jdm
Can this patch be backed out? It causes a regression: now the clipboard is never cached to the disk. To test (on Linux, but the problem is probably platform-independent): 1. Create a big file: python -c 'print("x x "*500000)' > /tmp/big.txt 2. Open the following test case: firefox-nightly /tmp/big 'data:text/html,<textarea style="width:90vw;height:90vh">' 3. In the first tab (with the repeated "x x"): Select all, copy text (Ctrl-A, Ctrl-C). 4. Go the second tab (input field): Ctrl-V. (This works as expected: The text is pasted.) 5. Ctrl-Z to empty the input field. 6. Remove /tmp/clipboardcache* 7. Ctrl-V again Expected (Nightly 60.0a1 2018-01-26): Nothing should be pasted because the file that backs the clipboard is removed. Actual (Nightly 60.0a1 2018-02-24): The data is pasted (indicating that the in-memory data is used). I discovered this issue when I was reviving my patches for bug 335545. This regression might be related to the nsITransferable initialization + IPC issue that I pointed to at the bottom of bug 1396224. Arthur/TOR, you might want to follow bug 1396224, which describes the bug that you are trying to solve here (plus more details about the cause of the bug).
Now the patches for bug 335545 have landed, the patch cannot be backed out by a revert (because the unit test has been modified). Given the lack of reply from the patch author, I'll direct the question from comment 7 to the reviewer: Josh, can/should the mPrivateData flag default to false again until bug 1396224 is fixed?
Sorry I didn't reply earlier! I'd be OK with backing this out while we work out the issue.
Thanks for the answer Arthur. I will submit a patch to flip the boolean again. Do you want to keep this bug open in order to flip the default again in the future, or should this be marked as a duplicate of bug 1396224 ?
Flags: needinfo?(josh) → needinfo?(arthuredelstein)
I'd be inclined to keep this bug open for continuity, since I imagine the final patch will be similar to the one I introduced before.
Alright. Anticipating the revert, I have re-opened the bug, assigned it to you and added the leave-open label so that the bug is not automatically closed when my revert lands.
Comment on attachment 8956753 [details] Bug 1433030 - Initialize nsTransferable::mPrivateData with false https://reviewboard.mozilla.org/r/225718/#review231672
Attachment #8956753 - Flags: review?(josh) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/87a34b1b1ad7 Initialize nsTransferable::mPrivateData with false r=jdm
Status: REOPENED → RESOLVED
Closed: Last year → 6 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1396224
You need to log in before you can comment on or make changes to this bug.