673301 - Reproducible crash on bookmarks drag&drop involving clipboard and compartments

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 1

[Lukas Blakk [:lsblakk] use ?needinfo]

Editing to add the crash signature, and also that this is on the sidebar, not in the left part of the view/list.

Comment 2

[Marco Bonardo [:mak]]

Hm I can't see a link or id to crash-stats :(

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

I copied in what was in the "Details" of the crash reporter - is there another way to get what you need?

Comment 4

[Marco Bonardo [:mak]]

the id from about:crashes would be useful, I can't reach the stack from those details...

Comment 5

[Lukas Blakk [:lsblakk] use ?needinfo]

Here we go:

http://crash-stats.mozilla.com/report/index/bp-fd1107e5-ec67-40d3-8ca8-5a2822110721
http://crash-stats.mozilla.com/report/index/bp-63b81048-5bb4-4b66-9d24-62f842110721
http://crash-stats.mozilla.com/report/index/bp-1e62230a-ca64-4f41-a6a0-08ad62110721
http://crash-stats.mozilla.com/report/index/bp-cdd3f8f8-d465-4722-8852-02c1a2110721

Comment 6

[Marco Bonardo [:mak]]

uh interesting, this has nothing to do with bookmarks, it's a drag&drop crash.

Cc-ing Enn

Comment 7

[Neil Deakin]

Attachment: null check data

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?

Comment 8

[Marco Bonardo [:mak]]

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 :(

Comment 9

[Marco Bonardo [:mak]]

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.

Comment 10

[Marco Bonardo [:mak]]

I had also to backout bug 475045 for a permaorange that appeared only on OSX and involving D&D of strings as well.

Comment 11

[Neil Deakin]

Attachment: patch, version 2

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.

Comment 12

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/6309d93cd072

Comment 14

[Marco Bonardo [:mak]]

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

Comment 16

[LegNeato]

http://hg.mozilla.org/releases/mozilla-aurora/rev/8ee4730f8a8a
http://hg.mozilla.org/releases/mozilla-beta/rev/04f7e81d82f1

Comment 18

[AndreiD[QA]]

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.

Comment 19

[AndreiD[QA]]

(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.

Comment 20

[Krisztian Paczari]

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)"

Comment 21

[Marcia Knous [:marcia]]

Adding Alex for the relnote piece in Comment 20.

Comment 22

[Scoobidiver (away)]

(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.