Closed Bug 673301 Opened 8 years ago Closed 8 years ago

Reproducible crash on bookmarks drag&drop involving clipboard and compartments

Categories

(Firefox :: Bookmarks & History, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Firefox 9
Tracking Status
firefox7 + fixed
firefox8 + fixed

People

(Reporter: lsblakk, Assigned: enndeakin)

References

Details

(Keywords: crash, verified-aurora, verified-beta, Whiteboard: [qa!])

Crash Data

Attachments

(1 file, 1 obsolete file)

Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0a2) Gecko/20110720 Firefox/7.0a2

Steps to reproduce:
1. Open Aurora with a clean profile
2. Go to Bookmarks -> Show all Bookmarks
3. In the Bookmarks Toolbar list of bookmarks try to drag one of the folders up or down to reorganize
4. Crash!

Expected result would be to re-order the list.
Editing to add the crash signature, and also that this is on the sidebar, not in the left part of the view/list.
Crash Signature: AdapterRendererIDs: 0x21b00,0x20400 Add-ons: rtmgmail@rememberthemilk.com:1.0.6,SQLiteManager@mrinalkant.blogspot.com:0.7.5,{972ce4c6-7e08-4474-a285-3208198ce6fd}:7.0a2,easy.app.tabs@phob.net:4,ffshare@mozilla.org:0.8.3 jid0-qBnIpLfDFa4LpdrjhAC6vBqN20Q@j…
Summary: Reproducible crash when trying to reorganize bookmarks in bookmarks toolbar → Reproducible crash when trying to reorganize bookmarks in bookmarks toolbar sidebar
Hm I can't see a link or id to crash-stats :(
Severity: major → critical
Keywords: crash
I copied in what was in the "Details" of the crash reporter - is there another way to get what you need?
the id from about:crashes would be useful, I can't reach the stack from those details...
uh interesting, this has nothing to do with bookmarks, it's a drag&drop crash.

Cc-ing Enn
Summary: Reproducible crash when trying to reorganize bookmarks in bookmarks toolbar sidebar → Reproducible crash on bookmarks drag&drop involving clipboard and compartments
Attached patch null check data (obsolete) — Splinter Review
This bug is caused because bookmarks appears to be setting text/plain drag data to some non-string. This patch fixes the crash on Mac at least.
 
Marco, the bookmarks code is still doing something wrong here. Can you debug where this might be happening?
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #547742 - Flags: review?(joshmoz)
Depends on: 673876
Sure, I filed bug 673876 to figure it out. It's strange it didn't happen in previous versions, we touched some code related to copy/paste but nothing about d&d :(
Attachment #547742 - Flags: review?(joshmoz) → review+
I had to backout this due to a permafailure on OSX

http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla-Inbound/1312577346.1312579429.5491.gz

and the failure is really interesting, looks like there is some other bug on OSX...
Notice there is some undefined since test_copypaste.html has some bogus usage of is(). The former fails the latter succeeds (OSX only):

55   function copySelectionToClipboard() {
56     documentViewer.copySelection();
57     is(clipboard.hasDataMatchingFlavors(["text/unicode"], 1,1), true);
58     is(clipboard.hasDataMatchingFlavors(["text/html"], 1,1), true);


34576 INFO TEST-PASS | /tests/content/base/test/test_copypaste.html | div5.innerHTML - "\n  T<textarea>     </textarea>\n" should equal "\n  T<textarea>     </textarea>\n"
34577 INFO TEST-PASS | /tests/content/base/test/test_copypaste.html | value of the textarea after the paste - "T " should equal "T "
NEXT ERROR 34578 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_copypaste.html | undefined - got false, expected true
34579 INFO TEST-PASS | /tests/content/base/test/test_copypaste.html | undefined - true should equal true
34580 INFO TEST-PASS | /tests/content/base/test/test_copypaste.html | Selection.toString - "" should equal ""
34581 INFO TEST-PASS | /tests/content/base/test/test_copypaste.html | div6.innerHTML - "div6" should equal "div6"
NEXT ERROR 34582 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_copypaste.html | undefined - got false, expected true
34583 INFO TEST-PASS | /tests/content/base/test/test_copypaste.html | undefined - true should equal true
34584 INFO TEST-PASS | /tests/content/base/test/test_copypaste.html | Selection.toString - "" should equal ""

.. and varoius others.
I had also to backout bug 475045 for a permaorange that appeared only on OSX and involving D&D of strings as well.
Attached patch patch, version 2Splinter Review
The cause of the failure is related to bug 564688. On Windows and Linux, when the text/unicode data is not valid an error is returned when the data is retrieved, which means that the flavour is added but the data is just empty. On Mac, the data for strings is determined when the data is placed on the clipboard rather than when it is retrieved, so we need to put something there to indicate that the flavour is present.

This version of the patch adds an empty string instead when the data cannot be converted.

Ideally, we would change the clipboard handling to validate the data when it is placed on the clipboard.
Attachment #552184 - Flags: review?(joshmoz)
Attachment #552184 - Flags: review?(joshmoz) → review+
http://hg.mozilla.org/mozilla-central/rev/6309d93cd072
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 9
Flags: in-testsuite+
Duplicate of this bug: 678145
Attachment #547742 - Attachment is obsolete: true
Comment on attachment 552184 [details] [diff] [review]
patch, version 2

I think we should backport this fix since users on osx are crashing easily just by dragging bookmarks, and the fix is small and safe enough
Attachment #552184 - Flags: approval-mozilla-beta?
Attachment #552184 - Flags: approval-mozilla-aurora?
Crash Signature: AdapterRendererIDs: 0x21b00,0x20400 Add-ons: rtmgmail@rememberthemilk.com:1.0.6,SQLiteManager@mrinalkant.blogspot.com:0.7.5,{972ce4c6-7e08-4474-a285-3208198ce6fd}:7.0a2,easy.app.tabs@phob.net:4,ffshare@mozilla.org:0.8.3 jid0-qBnIpLfDFa4LpdrjhAC6vBqN20Q@j… → [@ CoreFoundation@0x5712 ]
Duplicate of this bug: 681795
Attachment #552184 - Flags: approval-mozilla-beta?
Attachment #552184 - Flags: approval-mozilla-beta+
Attachment #552184 - Flags: approval-mozilla-aurora?
Attachment #552184 - Flags: approval-mozilla-aurora+
Whiteboard: [qa+]
Duplicate of this bug: 685052
There is no crash if following the steps in the description, on the current (up to date) Aurora and Firefox 7b5:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0a2) Gecko/20110908 Firefox/8.0a2
Setting this bug as Verified Fixed.
Status: RESOLVED → VERIFIED
(In reply to AndreiD[QA] from comment #18)
> There is no crash if following the steps in the description, on the current
> (up to date) Aurora and Firefox 7b5:
> Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101
> Firefox/7.0
> Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0a2) Gecko/20110908
> Firefox/8.0a2
> Setting this bug as Verified Fixed.

Also there is no crash if following the steps in the description on the latest Firefox Nightly build:
 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a1) Gecko/20110911 Firefox/9.0a1

So the issue is verified fixed at all Firefox development stages: nightly, aurora and beta.
Whiteboard: [qa+] → [qa!]
Please remove it from release notes now that it is fixed:
http://www.mozilla.org/en-US/firefox/9.0b5/releasenotes/
"Some users may experience a crash when moving bookmarks (see bug 681795)"
Adding Alex for the relnote piece in Comment 20.
(In reply to Krisztian Paczari from comment #20)
> Please remove it from release notes now that it is fixed:
> http://www.mozilla.org/en-US/firefox/9.0b5/releasenotes/
> "Some users may experience a crash when moving bookmarks (see bug 681795)"
See bug 701653.
You need to log in before you can comment on or make changes to this bug.