Last Comment Bug 754405 - Drag&Drop doesn't work with Tabs->Bookmarks and other things
: Drag&Drop doesn't work with Tabs->Bookmarks and other things
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: -- major (vote)
: seamonkey2.12
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
Depends on:
Blocks: 732027
  Show dependency treegraph
 
Reported: 2012-05-11 12:30 PDT by Frank Helk
Modified: 2012-05-29 09:58 PDT (History)
6 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
fixed


Attachments
(Av1) Fix 'titel' typo [Checked in: Comment 7 & 14 & 15] (1.13 KB, patch)
2012-05-26 23:07 PDT, Serge Gautherie (:sgautherie)
philip.chee: review+
jh: approval‑comm‑aurora+
jh: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Frank Helk 2012-05-11 12:30:04 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0) Gecko/20100101 Firefox/12.0
Build ID: 20120420145725

Steps to reproduce:

a) trying to drag & drop a tab to the bookmark toolbar
b) marking some text and try to drag&drop it into a seach box or similar


Actual results:

Drag & Drop doesn't work - nothing is dragged ... not even picked up.


Expected results:

a) Tab->Bokkmark should create a bookmark
b) Text should be entered into the search box
Comment 1 Matthias Versen [:Matti] 2012-05-12 00:07:18 PDT
Please try it again in the Firefox safemode
http://support.mozilla.com/en-US/kb/Safe+Mode
Comment 2 David E. Ross 2012-05-25 10:39:27 PDT
This is NOT a problem in SeaMonkey 2.9.1.  It thus appears to be a Toolkit regression.
Comment 3 NoOp 2012-05-26 13:41:39 PDT
Confirm.

SeaMonkey 2.10b(1&2) 32bit and 64bit linux, and 32bit Windows: Drag &
Drop icon URL from the address bar to the bookmarks toolbar does not
work. You can get a up/down grey arrow (the location
indicator?), but that's it. I know that the actual url data is being
copied as I can drag & drop to a search bar (duckduckgo or google) in an
open tab. I can also D&D to another application. I also cannot drag &
drop a tab to the bookmarks toolbar (as indicated in 'a) trying to drag & drop a tab to the bookmark toolbar'. No issues in SeaMonkey 2.9.1 (linux or Windows).
Comment 4 NoOp 2012-05-26 14:25:25 PDT
Added note: I cannot replicate on FireFox 12 (linux (32 or 64bit), WinXP, or Win7).
Comment 5 Philip Chee 2012-05-26 22:24:31 PDT
Error: ReferenceError: titel is not defined
Source file: resource://app/modules/PlacesUIUtils.jsm
Line: 312

http://hg.mozilla.org/comm-central/annotate/95c5300d3398/suite/common/src/PlacesUIUtils.jsm#l312

Regression caused by Bug 732027 - Port |Bug 575955 - Replace internal usage of old transactions shim, add a new toolkit test| to SeaMonkey
Comment 6 Serge Gautherie (:sgautherie) 2012-05-26 23:07:35 PDT
Created attachment 627534 [details] [diff] [review]
(Av1) Fix 'titel' typo
[Checked in: Comment 7 & 14 & 15]

[Approval Request Comment]
Regression caused by (bug #): bug 732027.
User impact if declined: broken features.
Risk to taking this patch (and alternatives if risky):
No risk, trivial.
Comment 7 Serge Gautherie (:sgautherie) 2012-05-26 23:08:32 PDT
Comment on attachment 627534 [details] [diff] [review]
(Av1) Fix 'titel' typo
[Checked in: Comment 7 & 14 & 15]

http://hg.mozilla.org/comm-central/rev/0be35ebd63a2
Comment 8 Frank Wein [:mcsmurf] 2012-05-27 00:44:06 PDT
Thanks for the quick fix!

Frank Helk: Did you test this with Firefox or SeaMonkey? Just wondering if Firefox still has this issue..
Comment 9 Serge Gautherie (:sgautherie) 2012-05-27 01:15:53 PDT
(In reply to Frank Wein [:mcsmurf] from comment #8)
> Just wondering if Firefox still has this issue..

That was the only occurrence (left) in c-c.
You mean it was not a typo of yours? Which changeset did you copy it from?
Comment 10 Frank Wein [:mcsmurf] 2012-05-27 01:39:16 PDT
No, it was a typo. But I wondered as the reporter reported this bug on Firefox (or did he actually meant to file this bug on SeaMonkey?).
Comment 11 Jens Hatlak (:InvisibleSmiley) 2012-05-27 03:38:14 PDT
Comment on attachment 627534 [details] [diff] [review]
(Av1) Fix 'titel' typo
[Checked in: Comment 7 & 14 & 15]

Note that bug 732027 didn't land on Aurora yet (but will have to, since it landed on Beta already), so we have a landing dependency here.

Speaking of the base bug, I feel we should no longer approve such "fixes" for Beta in the future. The base bug has caused two (!) regressions without much on the plus side of things. Beta should be for stabilization (fixing regressions, improving UX and the like), not jeopardizing the trust of our beta testers. While it's a valid goal to fix tests on all branches, I think we have to draw the line where we need to fix non-test code in order to fix a test. YMMV, but if it does, we should at least require more baking on trunk and more strict review in such cases.
Comment 12 Frank Wein [:mcsmurf] 2012-05-27 03:40:29 PDT
Actually I agree with you, I would not have requested approval for the branches for my patch. But Serge did, I guess I cannot prevent other people from requesting approval ;-)
Comment 13 Philip Chee 2012-05-27 04:16:09 PDT
Comment on attachment 627534 [details] [diff] [review]
(Av1) Fix 'titel' typo
[Checked in: Comment 7 & 14 & 15]

rs=me bustage fix
Comment 14 Jens Hatlak (:InvisibleSmiley) 2012-05-27 04:43:20 PDT
Comment on attachment 627534 [details] [diff] [review]
(Av1) Fix 'titel' typo
[Checked in: Comment 7 & 14 & 15]

http://hg.mozilla.org/releases/comm-beta/rev/6a1d10115b38
Comment 15 Philip Chee 2012-05-27 11:04:13 PDT
Pushed to comm-aurora:
http://hg.mozilla.org/releases/comm-aurora/rev/95a771b566da
Comment 16 Serge Gautherie (:sgautherie) 2012-05-29 09:37:23 PDT
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #11)

> Speaking of the base bug, I feel we should no longer approve such "fixes"

Base bug being SM bug 732027, right?

> for Beta in the future. The base bug has caused two (!) regressions without
> much on the plus side of things. Beta should be for stabilization (fixing
> regressions, improving UX and the like), not jeopardizing the trust of our

Ftr, my rationale is/was to have as similar code to FF (branches) as possible :-|

> beta testers. While it's a valid goal to fix tests on all branches, I think
> we have to draw the line where we need to fix non-test code in order to fix
> a test.

Note that the 2 test patches of that bug had already landed, with no issue.
Comment 17 Serge Gautherie (:sgautherie) 2012-05-29 09:39:40 PDT
(In reply to Serge Gautherie (:sgautherie) from comment #16)
> Note that the 2 test patches of that bug had already landed, with no issue.

I meant "my 2 patches : test + dead code removal".
Comment 18 Jens Hatlak (:InvisibleSmiley) 2012-05-29 09:58:22 PDT
(In reply to Serge Gautherie (:sgautherie) from comment #16)
> Base bug being SM bug 732027, right?

Yes.

> Ftr, my rationale is/was to have as similar code to FF (branches) as
> possible :-|

For test-only changes this is OK, but IMHO for code changes targeting comm-beta landing it's not. As I said, Beta is for stabilizing, and possibly regression normal operation (like happened here) just to fix some test or have more similar code is, IMHO, not a valid goal for that branch. If something breaks on Beta, we should be able to say "We gave our best to avoid such issues but still, this can happen. Sorry.", not "Oh sorry, I thought this was safe. Well, this is how we roll. Deal with it." After all, if the few beta users we have stop using that channel because we, despite the (relatively) long time the code has to stabilize (trunk -> Aurora -> Beta), needlessly regress things in between betas of the same target release, then this is entirely our fault.

> Note that the 2 test patches of that bug had already landed, with no issue.

Call it luck. As we can see from what happened here:
1. Even the smallest mistakes (typos, or shorthands typically used in FF land) can have a big effect.
2. Anyone can make mistakes. Patch authors, reviewers, approvers. I'm not excluding myself here; probably I was just lucky until now and was only partly responsible for bug 739049.

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