Last Comment Bug 725996 - New Tab Page should not respond to dragged bookmark folders
: New Tab Page should not respond to dragged bookmark folders
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 13
Assigned To: Tim Taubert [:ttaubert]
:
: Dão Gottwald [:dao]
Mentors:
Depends on:
Blocks: 455553 726009
  Show dependency treegraph
 
Reported: 2012-02-10 07:24 PST by Siddhartha Dugar [:sdrocking]
Modified: 2016-01-11 11:40 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 (3.29 KB, patch)
2012-03-12 07:22 PDT, Tim Taubert [:ttaubert]
dietrich: review+
Details | Diff | Splinter Review

Description Siddhartha Dugar [:sdrocking] 2012-02-10 07:24:43 PST
When I drag a bookmark folder to the thumbnail position it gets pinned there. But nothing happens when I click on its thumbnail. Maybe it shouldn't get pinned in the first place. Or, maybe we should open all bookmarks within the folder.
Comment 1 Thomas Ahlblom 2012-02-10 08:13:03 PST
Ouch! That makes my Firefox crash!

I have posted bug 726009 to keep this bug separated from the crash issue.
Comment 2 Tim Taubert [:ttaubert] 2012-03-12 07:22:52 PDT
Created attachment 604922 [details] [diff] [review]
patch v1

Trivial patch with test.
Comment 3 Dietrich Ayala (:dietrich) 2012-03-12 12:04:54 PDT
Comment on attachment 604922 [details] [diff] [review]
patch v1

Review of attachment 604922 [details] [diff] [review]:
-----------------------------------------------------------------

r=me w/ the comment below added.

::: browser/base/content/newtab/drag.js
@@ +106,5 @@
>     */
>    isValid: function Drag_isValid(aEvent) {
>      let dt = aEvent.dataTransfer;
> +    let mimeType = "text/x-moz-url";
> +    return dt && dt.types.contains(mimeType) && dt.getData(mimeType);

there's some implicitness here that should be commented on. what's going on here and why?
Comment 4 Tim Taubert [:ttaubert] 2012-03-12 19:08:30 PDT
(In reply to Dietrich Ayala (:dietrich) from comment #3)
> > +    let mimeType = "text/x-moz-url";
> > +    return dt && dt.types.contains(mimeType) && dt.getData(mimeType);
> 
> there's some implicitness here that should be commented on. what's going on
> here and why?

Added.

Pushed to fx-team:

https://hg.mozilla.org/integration/fx-team/rev/c1d32bb1b893
Comment 5 Tim Taubert [:ttaubert] 2012-03-13 03:36:35 PDT
https://hg.mozilla.org/mozilla-central/rev/c1d32bb1b893
Comment 6 Siddhartha Dugar [:sdrocking] 2012-03-14 01:29:08 PDT
Updating bug title from my understanding of the patch.
Comment 7 Tim Taubert [:ttaubert] 2012-03-14 01:38:24 PDT
Correct, thank you.
Comment 8 Virgil Dicu [:virgil] [QA] 2012-04-10 06:55:06 PDT
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20120409 Firefox/13.0a2
Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20120409 Firefox/14.0a1

Bookmark folders can't be dragged anymore in the new tab page. Setting to verified.

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