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]
:
:
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.
Comment 9 Neil Deakin (away until Oct 3) 2016-01-11 11:40:19 PST
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b550bf8631b5

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