Last Comment Bug 673301 - Reproducible crash on bookmarks drag&drop involving clipboard and compartments
: Reproducible crash on bookmarks drag&drop involving clipboard and compartments
Status: VERIFIED FIXED
[qa!]
: crash, verified-aurora, verified-beta
Product: Firefox
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: unspecified
: x86 Mac OS X
: -- critical with 1 vote (vote)
: Firefox 9
Assigned To: Neil Deakin
:
Mentors:
: 678145 681795 685052 (view as bug list)
Depends on: 673876
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-21 17:02 PDT by Lukas Blakk [:lsblakk] use ?needinfo
Modified: 2011-12-08 22:56 PST (History)
13 users (show)
enndeakin: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed


Attachments
null check data (3.18 KB, patch)
2011-07-22 10:49 PDT, Neil Deakin
jaas: review+
Details | Diff | Splinter Review
patch, version 2 (4.53 KB, patch)
2011-08-10 12:49 PDT, Neil Deakin
jaas: review+
asa: approval‑mozilla‑aurora+
asa: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Lukas Blakk [:lsblakk] use ?needinfo 2011-07-21 17:02:05 PDT
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 2011-07-21 17:03:32 PDT
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] 2011-07-21 17:14:35 PDT
Hm I can't see a link or id to crash-stats :(
Comment 3 Lukas Blakk [:lsblakk] use ?needinfo 2011-07-21 17:27:10 PDT
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] 2011-07-21 17:35:52 PDT
the id from about:crashes would be useful, I can't reach the stack from those details...
Comment 6 Marco Bonardo [::mak] 2011-07-22 05:27:50 PDT
uh interesting, this has nothing to do with bookmarks, it's a drag&drop crash.

Cc-ing Enn
Comment 7 Neil Deakin 2011-07-22 10:49:47 PDT
Created attachment 547742 [details] [diff] [review]
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] 2011-07-25 03:29:48 PDT
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] 2011-08-05 15:28:09 PDT
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] 2011-08-05 15:34:59 PDT
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 2011-08-10 12:49:49 PDT
Created attachment 552184 [details] [diff] [review]
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] 2011-08-19 03:08:22 PDT
http://hg.mozilla.org/mozilla-central/rev/6309d93cd072
Comment 13 Marco Bonardo [::mak] 2011-08-27 01:15:07 PDT
*** Bug 678145 has been marked as a duplicate of this bug. ***
Comment 14 Marco Bonardo [::mak] 2011-08-27 01:17:13 PDT
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 15 Scoobidiver (away) 2011-08-28 01:17:05 PDT
*** Bug 681795 has been marked as a duplicate of this bug. ***
Comment 17 XtC4UaLL [:xtc4uall] 2011-09-08 02:02:38 PDT
*** Bug 685052 has been marked as a duplicate of this bug. ***
Comment 18 AndreiD[QA] 2011-09-09 07:25:12 PDT
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] 2011-09-12 04:52:07 PDT
(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 2011-12-08 12:37:01 PST
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 - use ni] 2011-12-08 13:27:26 PST
Adding Alex for the relnote piece in Comment 20.
Comment 22 Scoobidiver (away) 2011-12-08 22:56:56 PST
(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.

Note You need to log in before you can comment on or make changes to this bug.