Closed
Bug 455694
Opened 16 years ago
Closed 13 years ago
Implement animation for tab reordering and detaching (via drag and drop)
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
RESOLVED
DUPLICATE
of bug 674925
Firefox 8
People
(Reporter: klaas1988, Unassigned)
References
(Depends on 2 open bugs, Blocks 8 open bugs, )
Details
(Keywords: ux-userfeedback, Whiteboard: [backed out in bug 690227 -- see bug 674925][parity-safari][parity-chrome][parity-ie])
Attachments
(3 files, 40 obsolete files)
6.02 KB,
text/html
|
Details | |
5.84 KB,
patch
|
Details | Diff | Splinter Review | |
60.13 KB,
patch
|
Details | Diff | Splinter Review |
When you drag and drop a tab on the tabbar in Safari and Chrome you see a nice animation, the tab is moved to the place where the mouse cursor is before you drop it. Currently Firefox only shows a small arrow to indicate where the tab will be dropped. When you drag the the tab outside the toolbar in those browsers (which "detaches" the tab) the tab changes into a thumbnail of that tabs content. This bug is for implementing the same behavior in Firefox. From bug 225680 comment #79: > The animation stuff (was looking at Safari's tab drag today) might be a little > tough :), but we could split that out as a separate bug and take a look to see > what's possible. So here it is!
Reporter | ||
Updated•16 years ago
|
Whiteboard: [parity-safari] [parity-chrome]
Reporter | ||
Comment 1•16 years ago
|
||
Maybe the possibility to use "arbitrary elements as paint servers for css backgrounds" that roc plans to add, could be used to create the detach tab thumbnail. This could be used instead of something like canvas draWindow. See: http://weblogs.mozillazine.org/roc/archives/2008/07/the_latest_feat.html
Comment 2•16 years ago
|
||
As far as I know, Mano has the thumbnail preview working in bug 225680 now. What needs to be done is animating the other tabs on the tabstrip so that they slide gracefully as the dragged tab is moved left/right. (I'll be honest; I'm finding it hard to decode what parts of this are bug 225680 and what parts are here.)
Updated•16 years ago
|
Hardware: PC → All
Comment 6•15 years ago
|
||
This looks like a duplicate of this: https://bugzilla.mozilla.org/show_bug.cgi?id=410972
Updated•15 years ago
|
Comment 8•14 years ago
|
||
Any chances to get this finalized in Firefox 4 ? ;)
Comment 9•14 years ago
|
||
(In reply to comment #8) > Any chances to get this finalized in Firefox 4 ? ;) I don't see why not. It should block the final release at least
Comment 10•14 years ago
|
||
(In reply to comment #9) > (In reply to comment #8) > > Any chances to get this finalized in Firefox 4 ? ;) > I don't see why not. It should block the final release at least The animated mockups are here : http://blog.stephenhorlander.com/2010/01/29/tab-animation/
Comment 11•14 years ago
|
||
Shouldn't this be split up into two bugs? Tab dragging animation would be easier to handle first, then tab dropping.
Comment 12•14 years ago
|
||
Demo: http://people.mozilla.com/~fyan/prototypes/tabslide.html
Comment 13•14 years ago
|
||
(In reply to comment #12) > Demo: > http://people.mozilla.com/~fyan/prototypes/tabslide.html That looks very nice. I really look forward to seeing this in a browser.
Comment 14•14 years ago
|
||
This demo is the most beautiful thing that I've ever seen since the first time that I saw Tab Candy! I'd really like to see this in Firefox 4. Please keep up the good work! I'll try to lurk around in the bug and make myself useful by giving drive-by reviews and such.
Comment 15•14 years ago
|
||
Frank, you're the man. How are you going to handle the case where you drag a tab out of an almost-overflowing tabstrip? Ideally the other tabs would grow back with animation, and you can't do that just with transforms.
Comment 16•14 years ago
|
||
Thanks guys. :) When I have time, there are other bits that I'd like to add to the prototype, including overflow handling and a smoother detach animation. Code contributions are welcome. ;) (In reply to comment #15) > How are you going to handle the case where you drag a tab out of an > almost-overflowing tabstrip? Ideally the other tabs would grow back with > animation, and you can't do that just with transforms. This is debatable, but my intuition is that the tabs should not grow back until the detached tab is dropped. The UX advantage in bug 465086 of deferring the resize does not apply here, but I do think that it is less jarring anyway.
Comment 17•14 years ago
|
||
(In reply to comment #16) > Created attachment 463283 [details] > my prototype (mirror of link in comment 12) Holy crap that is awesome! I'm even going to request blocking because of the awesomeness of it bringing some huge polish to rearranging tabs.
blocking2.0: --- → ?
Comment 18•14 years ago
|
||
What does it take to go from prototype to patch, here? (So sad that it's not "same technology, same difference!") I wouldn't hold the release on this, but have wanted this since Firefox 2, so you can bet your bippy that I'd approve the heck out of a patch that came along with tests and no performance impact.
Comment 19•14 years ago
|
||
how will this work when ctrl is pressed for duplicating the tab?
Comment 20•14 years ago
|
||
I made the mistake of making a crack about how it was sad that it's such a pain to do this kind of stuff in XUL in front of Neil Deakin, and he made this XUL version/translation. Frank: does this help you get to a patch? (Neil: you are awesome)
Comment 21•14 years ago
|
||
(In reply to comment #20) > Created attachment 463754 [details] > tab dragging in XUL > > Frank: does this help you get to a patch? Thanks for the translation, Neil. Beltzner, my HTML prototype used display: -moz-box, which is the XUL box model, so I had an idea of where to go next, but having it already coded for me is always fantastic :) Before I put together a patch, I have some more ideas up my sleeve. ;)
Comment 22•14 years ago
|
||
(In reply to comment #21) > (In reply to comment #20) > > Created attachment 463754 [details] [details] > > tab dragging in XUL > > > > Frank: does this help you get to a patch? > > Thanks for the translation, Neil. > > Beltzner, my HTML prototype used display: -moz-box, which is the XUL box model, > so I had an idea of where to go next, but having it already coded for me is > always fantastic :) > > Before I put together a patch, I have some more ideas up my sleeve. ;) Perfect is the enemy of good! I want this piece already!
Comment 23•14 years ago
|
||
Don't think I can block on this, but I really really want it; I will continue to help it forward as I can, up to and including approvals!
blocking2.0: ? → -
Updated•14 years ago
|
Keywords: ux-feedback
Comment 25•14 years ago
|
||
I'm a bit new to this game, so I don't really know when feature freeze is, but is this going to make it? Anyone working on this?
Comment 26•14 years ago
|
||
(In reply to comment #25) > I'm a bit new to this game, so I don't really know when feature freeze is, but > is this going to make it? Anyone working on this? There's no assignee, so nobody is working on this.
Comment 27•14 years ago
|
||
(In reply to comment #23) > Don't think I can block on this, but I really really want it; I will continue > to help it forward as I can, up to and including approvals! Isn't it a key part of the Fx4 facelift as many changes happenned to tabs with Fx4 final as a target : UX changes, position (on top) and the animation when openning/closing tab ? At least could we just implement animation for reordering tab for Fx4 as I guess it's a more common action and that I saw in a previous nightly (on Linux and mac) that the "new tab's position" indicator was truncated because of tab-on-top ? (yes mean reop bug 410972 and target it to Fx4)
Comment 28•14 years ago
|
||
I think it's noteworthy to look at how IE9 handles the animation for tear off tabs. It's quite interesting
Comment 29•14 years ago
|
||
(In reply to comment #28) > I think it's noteworthy to look at how IE9 handles the animation for tear off > tabs. It's quite interesting Could you catch that and send it (blog, attachement, ...) or at least explain what IE9 do (I only have OSX and Linux desktops) please ?
Comment 30•14 years ago
|
||
[parity-IE] should be added, right ? ;-)
Comment 31•14 years ago
|
||
In IE9 beta, tab reordering shows a sliding animation just like the prototype attached to this bug, and like Chrome and Safari.
Whiteboard: [parity-safari] [parity-chrome] → [parity-safari][parity-chrome][parity-ie]
Comment 32•14 years ago
|
||
As this bug cover reodering and tear-off animations of tabs, I propose to change "animation" to animationS" in the title.
Comment 33•14 years ago
|
||
To actually make this searchable, how about "Implement animations for tab reordering and tab tearing".
Comment 34•14 years ago
|
||
(In reply to comment #29) > Could you catch that and send it (blog, attachement, ...) or at least explain > what IE9 do (I only have OSX and Linux desktops) please ? The rest of the UI elements disappear so that only the tab and page content are visible. There's no animation like in Chrome where the thumbnail's small while dragging. http://img189.imageshack.us/img189/989/clipboard01dd.png Very seamless (I'd make a video if I could find a program that makes webm video)
Comment 35•14 years ago
|
||
(In reply to comment #33) > To actually make this searchable, how about "Implement animations for tab > reordering and tab tearing". So, should block bug 466196 ;-)
Updated•14 years ago
|
Updated•14 years ago
|
Summary: Implement animation for drag & dropping of tabs → Implement animation for tab reordering (by dragging and dropping)
Comment 36•14 years ago
|
||
Video of IE9 beta tab animations per comment 34
Comment 37•14 years ago
|
||
I think this video is more related to Bug 485105 and Bug 485105 .
Comment 38•14 years ago
|
||
Sorry, and to Bug 504847 .
Comment 39•14 years ago
|
||
is something going to happen with this bug?
Comment 40•14 years ago
|
||
(In reply to comment #39) > is something going to happen with this bug? After Firefox 4, I might pick it up.
Comment 41•14 years ago
|
||
How close are we to finishing this, Frank? Any chance we could pick it up before Firefox 4, or is it pretty far away?
Comment 42•14 years ago
|
||
(In reply to comment #41) > How close are we to finishing this, Frank? Any chance we could pick it up > before Firefox 4, or is it pretty far away? A dogfood-able patch should be quickly do-able. My concern is about polish and review, but if you and the team think this is something that we'd really want for Firefox 4 (and I do), then I'm definitely willing to invest time in it.
Comment 43•14 years ago
|
||
Definitely a big polish win for an area where we currently look clunky. If you can work out a dogfoodable patch we can toss to tryserver and play with in that build I think it'd be worthwhile, yes.
Updated•14 years ago
|
Assignee: nobody → fryn
Status: NEW → ASSIGNED
Comment 44•14 years ago
|
||
Mostly dogfoodable work-in-progress. There will be many changes to come. I am well aware of the issues, so no need to report anything yet. I'll throw up tryserver builds once this is closer to feature complete.
Updated•14 years ago
|
Updated•14 years ago
|
Blocks: cuts-tabs
Summary: Implement animation for tab reordering and detaching (by dragging and dropping) → Implement animation for tab reordering and detaching (via drag and drop)
Comment 46•14 years ago
|
||
Does this bug count as a new feature? As in, could it still land for firefox 4.0 since feature freeze has happened?
Comment 47•14 years ago
|
||
See comments #42 and #43 right above you?
Comment 48•14 years ago
|
||
Yeah but those comments were before beta 7...
Comment 49•14 years ago
|
||
(In reply to comment #48) > Yeah but those comments were before beta 7... Tabs is toolbar is currently in tryserver. There's also an issue with drop markets on Mac, thus the plan is to get this, along with the tabs in toolbar for systems that support it, in time for the next beta.
Comment 50•14 years ago
|
||
What is "tabs in toolbar" ? :S
Comment 51•14 years ago
|
||
(In reply to comment #49) > Tabs is toolbar is currently in tryserver. There's also an issue with drop > markets on Mac, thus the plan is to get this, along with the tabs in toolbar > for systems that support it, in time for the next beta. There is zero chance of this making beta 8. Reordering animations will only partially resolve dropmarker issues. Cropped dropmarkers will still appear when dragging links to the tab bar, until we address that too.
Comment 52•14 years ago
|
||
(In reply to comment #50) > What is "tabs in toolbar" ? :S Sorry meant tabs in titlebar (bug 572160). (In reply to comment #51) > (In reply to comment #49) > > Tabs is toolbar is currently in tryserver. There's also an issue with drop > > markets on Mac, thus the plan is to get this, along with the tabs in toolbar > > for systems that support it, in time for the next beta. > > There is zero chance of this making beta 8. Reordering animations will only > partially resolve dropmarker issues. Cropped dropmarkers will still appear when > dragging links to the tab bar, until we address that too. My apologies for the misinformation and thank you for the correction.
Comment 53•14 years ago
|
||
Nomming for blocking again. This has been discussed repeatedly as being a pretty big UX win that's highly wanted; I think we should either bite the bullet and call this one of the rate UX-polish blockers, or cut our losses now and punt. Seems like this isn't too invasive of a change, code-wise, and Frank and I have discussed how to break up the work into contained, bite-size steps such that we can get incremental improvements to tab reordering/detaching as we go. But since it's a sequence, changes some existing UI behaviors, and will have a couple of dependencies (to be filed shortly) we should discuss this up front.
blocking2.0: - → ?
Comment 54•14 years ago
|
||
The final patch for his bug will depend on the mouse coordinates being available during drag-and-drop source node events for content to be able to calculate whether the tab will be detached upon releasing the mouse. Thus, I am adding bug 505521 as a dependency. The coordinates only need to be available for those events for chrome code. If there is an easier way to obtain mouse coordinates for drag events, let me know.
Depends on: 505521
Comment 55•14 years ago
|
||
I really hope that there's an easier way, because that dependency looks ugly. :/ Enn?
Comment 56•14 years ago
|
||
I don't think exposing mouse coordinates for those events is a particularly complex dependency, but Enn would know better. Aside from that, my patch is almost feature-complete now, so once we can resolve that dependency, I could have this up for review in a week.
Comment 57•14 years ago
|
||
But what coordinates do you want? Relative to what? Which event?
Comment 58•14 years ago
|
||
(In reply to comment #57) > But what coordinates do you want? Relative to what? Which event? Mouse coordinates relative to screen left and top (i.e. screenX and screenY) would be easiest to use, but since the positions of the node, window, etc. are already known, a different origin would work too. I'd need it for the "drag" event. (Coordinates are available for "dragover", but that's not helpful when the drag moves outside of the window.)
Comment 59•14 years ago
|
||
I have a patch of the detach animation working well enough using Enn's current patch for bug 533460. Will upload with more information once I clean it up.
Comment 60•14 years ago
|
||
This uses the prototype patch from bug 533460, so apply that patch first. It is *feature-complete* and absolutely dogfoodable. I'll link to tryserver builds once they're up. The patch might look large, but a lot of it is just moving or deleting code. Remaining known issues: - There shouldn't be a small unfilled rectangle next to the mouse cursor during drags. This can be fixed with a small change to the patch for bug 533460. - Dragging a tab over the bookmarks menu button either shouldn't open the menu or should keep the drag image above the menu. - Dragging a tab over a tab strip scroll button should scroll the tab strip. More eye candy that we could do in the future in followup bugs: - Smoothly animate the movement of background tabs. - Dragging an app tab to the normal tab region or dragging a normal tab to the app tab region moves the dragged tab instead of using the drop indicator. - Detaching a tab animates the drag image out of the tab strip. - The detachment threshold is only active when detaching a tab and not when reattaching it. - Dragging a tab to another window animates the drag image into the tab strip instead of using the drop indicator. - Dragging a tab to the right of its sibling tabs and the new tab button continues to move the tab within the bounds of the tab strip.
Attachment #492890 -
Attachment is obsolete: true
Comment 61•14 years ago
|
||
Tryserver builds: https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5f971390c796/ Addenda: Known issues: - The drop location of a tab should be dependent upon the element's location not the mouse location. In the above eye candy section, the statements refer to desired behavior, not current behavior.
Comment 62•14 years ago
|
||
Doesn't seem to work on ubuntu, can't move tabs at all. The only thing that happens is some icon appears briefly near the mouse pointer when I try to move a tab.
Comment 63•14 years ago
|
||
Will there be any Windows builds? I want to test this feature on Win7..
Comment 64•14 years ago
|
||
(In reply to comment #63) > Will there be any Windows builds? I want to test this feature on Win7.. They are in the previously provided link....https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5f971390c796/tryserver-win32-debug/
Comment 65•14 years ago
|
||
That's the debug build...
Comment 66•14 years ago
|
||
While awesome, this is a new feature. We'd like to ship Firefox 4 before the singularity occurs. Blocking-.
blocking2.0: ? → -
Comment 67•14 years ago
|
||
(In reply to comment #66) > While awesome, this is a new feature. We'd like to ship Firefox 4 before the > singularity occurs. Blocking-. Comment 41 and comment 43 infer that this will make it into Firefox 4 despite the feature freeze?
Comment 68•14 years ago
|
||
If the patch is ready and safe, we'd take it. The blocking status says that we won't hold back the release for it if it's not ready.
Comment 69•14 years ago
|
||
Ah ok, thanks for the clarification.
Comment 70•14 years ago
|
||
Fixed all known issues within the scope of _this_ bug, including the pause after rearranging tabs. The two following issues should be fixed in their respective bugs: - There shouldn't be a small unfilled rectangle next to the mouse cursor during drags. This can be fixed in bug 533460. - Dragging a tab over the bookmarks menu button either shouldn't open the menu or should keep the drag image panel above the menu (could also be fixed in bug 533460). I'll push this to tryserver when I'm confident that it's ready for a first review.
Attachment #495767 -
Attachment is obsolete: true
Attachment #495804 -
Flags: feedback?(dolske)
Comment 71•14 years ago
|
||
(In reply to comment #63) > Will there be any Windows builds? I want to test this feature on Win7.. Tryserver isn't instantaneous. The Windows builds with WIP v2 are up now. (In reply to comment #65) > That's the debug build... Debug builds run just fine for trying out code and features. (In reply to comment #62) > Doesn't seem to work on ubuntu, can't move tabs at all. The only thing that > happens is some icon appears briefly near the mouse pointer when I try to move > a tab. Neil explained in bug 533460 that his proof-of-concept does not work on Linux. I'll ask him about it. In the meantime, I'll code in a blank drag image for Linux.
Comment 72•14 years ago
|
||
> Debug builds run just fine for trying out code and features.
If you can run debug builds, yes. (I don't think much of the triers can.)
Comment 73•14 years ago
|
||
Attachment #495804 -
Attachment is obsolete: true
Attachment #496124 -
Flags: review?(gavin.sharp)
Attachment #496124 -
Flags: review?(dao)
Attachment #495804 -
Flags: feedback?(dolske)
Comment 74•14 years ago
|
||
(In reply to comment #73) > Created attachment 496124 [details] [diff] [review] > patch v1 The entire patch was refactored. Small fixes here and there. Despite the size of the patch, a lot of it is code removal. It uses a black drag image on Linux, since the patch for bug 533460 has yet to support Linux. Apply the patch for bug 533460 first. I'll post tryserver links once available.
Comment 75•14 years ago
|
||
Thank you for the test builds! Works perfectly on Windows 7 despite one fact... I'm not joking. I really can see it. The tab re-ordering animation is extremely laggy and slow when using Microsoft Arc mouse (especially when moving the tab quickly). When I'm using touchpad with the same browser window with the same set of tabs, animation is nice and smooth. The CPU usage seems to be similar in both cases, IntelliPoint is not installed. If you'll decide to take it serious, I'll be happy to try to help with debugging.
Updated•13 years ago
|
Whiteboard: [parity-safari][parity-chrome][parity-ie] → [parity-safari][parity-chrome][parity-ie][target-next-beta]
Updated•13 years ago
|
Whiteboard: [parity-safari][parity-chrome][parity-ie][target-next-beta] → [parity-safari][parity-chrome][parity-ie][target-betaN]
Comment 76•13 years ago
|
||
Attachment #496124 -
Attachment is obsolete: true
Attachment #497424 -
Flags: review?(gavin.sharp)
Attachment #497424 -
Flags: review?(dao)
Attachment #496124 -
Flags: review?(gavin.sharp)
Attachment #496124 -
Flags: review?(dao)
Comment 77•13 years ago
|
||
Comment on attachment 497424 [details] [diff] [review] WIP v5 Bouncing from gavin to dao... Dao, could you at least a first-pass review done on this, so we can get a better sense of any issues remaining here?
Attachment #497424 -
Flags: review?(gavin.sharp)
Comment 78•13 years ago
|
||
Comment on attachment 497424 [details] [diff] [review] WIP v5 Couple of superficial comments from a quick skim through... I'll start a real review if dao doesn't have time. >+++ b/browser/base/content/tabbrowser.css ... >+.tab-drag-image { >+ -moz-transform: translate(1px); >+} IIRC, you said this was a workaround to avoid dispatching events to the drag-image panel? This is worth a comment in the CSS, and reference to a bug number if applicable. >+ <method name="_updateTabDetachState"> ... >+ let bo = this.mTabstrip.boxObject; How about "box"?
Comment 79•13 years ago
|
||
> >+ <method name="_updateTabDetachState">
> ...
> >+ let bo = this.mTabstrip.boxObject;
>
> How about "box"?
I just took that from:
- if (eX > wX && eX < (wX + window.outerWidth)) {
- let bo = this.mTabstrip.boxObject;
but I can definitely change it. :)
I'll address these comments in the next patch once this gets a more thorough review (to avoid excessive versions).
Comment 80•13 years ago
|
||
(In reply to comment #68) > If the patch is ready and safe, we'd take it. The blocking status says that we > won't hold back the release for it if it's not ready. So, if this is not implemented in time for Firefox 4 official release (I'm talking NON Release Candidates), will you then aim for the first official patch (say 4.0.1)? I would really like to see this feature implemented by the time Firefox 4 is out of beta status, because I am not the only one who moves tabs around in full-screen mode, and with the 1/8/11 build there is now no way to tell where tabs are moving to since the tabs don't animate over and the black arrow is invisible in full screen mode.
Comment 81•13 years ago
|
||
If this isn't able to be implemented by release, bug 572160 would have to be backed out.
Comment 82•13 years ago
|
||
(In reply to comment #81) > If this isn't able to be implemented by release, bug 572160 would have to be > backed out. This is nonsense. The drop indicator needs to be fixed regardless of this bug.
Comment 83•13 years ago
|
||
So this bug will block final Firefox 4?
Comment 84•13 years ago
|
||
He said that the drop indicators have to be fixed - REGARDLESS of this bug here. So why should it block?
Comment 85•13 years ago
|
||
o... it seems that I misunderstood what he said. my bad. Sorry.
Comment 86•13 years ago
|
||
I was trying the patch, it looks good but it's a bit buggy. 1. looks like you completely killed the possibility to drag a tab to bookmarks toolbar or bookmarks menu). Is there a specific reason for this? Unfortunately I use this gesture a lot :( 2. when dragging a tab across windows, often the old arrow indicator appears instead of the new tab. Sometimes the new tab appears but on release a new window is spawned instead of adding the tab.
Comment 87•13 years ago
|
||
(In reply to comment #86) > I was trying the patch, it looks good but it's a bit buggy. > > 1. looks like you completely killed the possibility to drag a tab to bookmarks > toolbar or bookmarks menu). Is there a specific reason for this? Unfortunately > I use this gesture a lot :( I killed this, because once tabs become something that the user is visually dragging, the tab should no longer be overloaded as an affordance that represents the tab's current page. For example, dropping the tab on a web page's text field shouldn't insert the tab's current URL in that field; rather, it should have visually indicated that it would be detached and then be detached upon drop. The location bar favicon's visual promixity to the URL and hierarchical association with the current location makes it an affordance that represents well the current page. Dragging the location bar favicon naturally inserts the current page, whereever it is dropped. Having the favicon inside the identity block does muddle this association, but we are moving the favicon outside of the identity block (bug 610048). > 2. when dragging a tab across windows, often the old arrow indicator appears > instead of the new tab. I'm leaving this animation (replacing the drop indicator with a tab when dragging tabs between windows) to a followup, since it makes this bug much more complicated and dragging between windows is not the common case. > Sometimes the new tab appears but on release a new > window is spawned instead of adding the tab. I don't see this. STR? With the revised schedule for Firefox 4 and amount of work in both my, Dao's, and Neil's hands, I don't think it makes sense to continue to push for getting this into Firefox 4. It will be one of the first things to tackle for the subsequent release.
Updated•13 years ago
|
Attachment #497424 -
Flags: review?(dao)
Comment 88•13 years ago
|
||
(In reply to comment #87) > With the revised schedule for Firefox 4 and amount of work in both my, Dao's, > and Neil's hands, I don't think it makes sense to continue to push for getting > this into Firefox 4. It will be one of the first things to tackle for the > subsequent release. So, do you mean the users won't see tab movement animations in Firefox (like is already available in IE 9 Beta) until Firefox 5.0 or within one of the first official patches for 4.0?
Comment 89•13 years ago
|
||
I really feel this should be a blocking bug for Firefox 4, because I can't be the only one who moves tabs around in full screen and can't tell where I am moving them to. Before you put tabs in line with the menu bar in the 1/8/11 Minefield build, this was never an issue because the tabs always remained below the menu bar in full and non-full screen modes.
Comment 90•13 years ago
|
||
(In reply to comment #88) > So, do you mean the users won't see tab movement animations in Firefox (like is > already available in IE 9 Beta) until Firefox 5.0 or within one of the first > official patches for 4.0? It won't be in 4.0 -- that's the right call, there's a ton of awesomeness in 4.0 compared to 3.6 and we need to ship. What release this patch hits is something we can consider after 4.0 ships.
Comment 91•13 years ago
|
||
All I want is some indication in full-screen when moving tabs. The black arrow is now hidden when in full-screen because it is set to be drawn at horizontal position 0, which can't be seen if the tabs are ALSO at position 0.
Comment 92•13 years ago
|
||
(In reply to comment #91) > All I want is some indication in full-screen when moving tabs. The black arrow > is now hidden when in full-screen because it is set to be drawn at horizontal > position 0, which can't be seen if the tabs are ALSO at position 0. That's being handled by a separate bug.
Comment 93•13 years ago
|
||
(In reply to comment #92) > (In reply to comment #91) > > All I want is some indication in full-screen when moving tabs. The black arrow > > is now hidden when in full-screen because it is set to be drawn at horizontal > > position 0, which can't be seen if the tabs are ALSO at position 0. > > That's being handled by a separate bug. Right, bug 572160, which is listed as blocking.
Comment 94•13 years ago
|
||
(In reply to comment #87) > The location bar favicon's visual promixity to the URL and hierarchical > association with the current location makes it an affordance that represents > well the current page. So, the suggestion is to switch to the tab I want to drag, and drag the favicon instead. Sure, this works, it's just a bit longer (if I want to drag a inactive tab) and probably far from what a user will try to do at first, but it's just matter of getting used to it, as many other changes. Mostly I was guessing if there were also technical reasons to disable drop to bookmarks, or was just a design decision. > > Sometimes the new tab appears but on release a new > > window is spawned instead of adding the tab. > > I don't see this. STR? Well, sometimes it worked correctly, sometimes not, so it's hard to give steps :( Mostly I was trying to drop a tab from window A to the first position in window B, and about 1/4 of the times the tab was detached rather than dropped in that window.
Comment 95•13 years ago
|
||
I can understand this is hard work, but this is something you use everyday, dozens of times – and I think it’s a significant drawback in polish that makes firefox look outdated and clumsy. If not for 4.0 I hope this will be in 4.0.1 or another minor update, Firefox 4.5 a year later.
Comment 96•13 years ago
|
||
(In reply to comment #94) > (In reply to comment #87) > > The location bar favicon's visual promixity to the URL and hierarchical > > association with the current location makes it an affordance that represents > > well the current page. > > So, the suggestion is to switch to the tab I want to drag, and drag the favicon > instead. Sure, this works, it's just a bit longer (if I want to drag a inactive > tab) and probably far from what a user will try to do at first, but it's just > matter of getting used to it, as many other changes. > Mostly I was guessing if there were also technical reasons to disable drop to > bookmarks, or was just a design decision. A design decision. Since to the user, the tab has already visually moved out of the tab strip and become the unit that the user is dragging, it no longer makes as much sense to allow dropping the current URL on any target that will take it. It produces way too much drop targets, which the user has to avoid when wanting to detach a tab, and also looks odd to bring back the tab in the tab strip after having dropped it into, say, a bookmarks folder. A simple example of how this could go wrong: 1. The user begins to drag a tab downward (tabs-on-top). 2. Approximately somewhere vertically within the location bar, the tab visually indicates that it will be detached upon release and is no longer within the tab strip. 3. If the user releases the tab over the location bar, the browser will load that tab's current page in the current tab. (When dragging the current tab, this effectively reloads the page.) Then the tab reappears in the tab strip. 4. If the user releases the tab over the bookmarks toolbar, a bookmark will be created. Then the tab reappears in the tab strip. 5. If the user releases the tab in a web page text field, the tab's current URL will be inserted into the text field. The tab then reappears in the tab strip. If we allow the tab to be an affordance to drag the page URL, the user must avoid all these elements when attempting to detach a tab. If we want to indicate precisely that the tab's URL will be dropped and not the tab itself, it would result in visual jerkiness as the tab disappears and reappears in the tab strip or morphs shape during the drag.
Comment 97•13 years ago
|
||
(In reply to comment #87) > I killed this, because once tabs become something that the user is visually > dragging, the tab should no longer be overloaded as an affordance that > represents the tab's current page. I disagree and think this is a mistake. Adding this functionality shouldn't remove other functionality. The user shouldn't and generally doesn't care about the mechanisms behind the scenes. The ability to grab a tab and either reorder them or drag it to the bookmarks menu or linkbar shouldn't be mutually exclusive, especially since this isn't adding the reorder ability, but modifying that. If I grab a desktop icon, I can drag it to another position (reorder), drag it into a folder (reorder/move, akin to moving between windows, or bookmarking), or drag it to the system's recycle bin/trash (delete), or if it's a file icon as opposed to an application icon, I drag the file onto an application icon, and open that file in that application. I can perform multiple types actions with the same user action, I'm not artificially limited. In Windows 7, the location bar in Explorer serves multiple purposes. Itshows my current location and the hierarchy to that location; each branch in the location tree is both a quick "go to" button, and a flyout list for navigating; the entire location is also editable text like a command line. Simply because it's a graphical element didn't mean the developers had to artificially limit it's functionality. As far as the user is concerned, the tab is an object representing a page, and they want to be able to reorder that page, bookmark that page, create a button on the linkbar, heck, I can't even see why they shouldn't be able to drag the tab to the PRINT button if they have it on the toolbar. Once you make an object dragable, you need to be prepared to create interactions that the user expects, not retrain the user because you dislike the metaphor. Users think, "this will go where I drag it." When you put a paper in a file, it's filed; when you put it in a shredder, it's shredded; when you put it on your desk, it stays there. You don't need to change the mode of the paper depending where its going. > For example, dropping the tab on a web > page's text field shouldn't insert the tab's current URL in that field; rather, > it should have visually indicated that it would be detached and then be > detached upon drop. Why can't it do both? I grant you replacing the current URL in the URLbar with the current URL is pointless, but the concept that the dragable object interacts with whatever object it is being dragged to in the manner the destination object operates is more intuitive than saying "To interact in method A, drag page-object 1. To interact in method B, drag page-object B."
Comment 98•13 years ago
|
||
(In reply to comment #97) > The user shouldn't and generally doesn't care about > the mechanisms behind the scenes. This has nothing to do with implementation details. It's a design decision. > If I grab a desktop icon, I can drag it to another position (reorder), drag it > into a folder (reorder/move, akin to moving between windows, or bookmarking), > or drag it to the system's recycle bin/trash (delete), or if it's a file icon > as opposed to an application icon, I drag the file onto an application icon, > and open that file in that application. I can perform multiple types actions > with the same user action, I'm not artificially limited. That works fine, because you're moving a ghosted drag image of the icon, not the icon itself. We move a ghosted tab drag image right now in Firefox 3.6. What we want to do is move the tab itself.
Comment 99•13 years ago
|
||
My comment is in regards to "my prototype (mirror of link in comment 12)". This is exactly how it should work. However, the tabs should not be able to be dragged past the left and right edges of the screen when ready to be implemented in the Firefox code.
Comment 100•13 years ago
|
||
(In reply to comment #98) > This has nothing to do with implementation details. It's a design decision. I understand, but that weakens your argument. > That works fine, because you're moving a ghosted drag image of the icon, not > the icon itself. We move a ghosted tab drag image right now in Firefox 3.6. > What we want to do is move the tab itself. I don't think that matters. If they drop it on the bookmarks bar or menu, then the tab can snap back to the previous position, and then the menu or bar will behave as it does in Fx3.x.
Comment 101•13 years ago
|
||
Did I read right, users will no longer be able to drag tabs in order to bookmark them? Neither to the bookmarks toolbar nor the bookmarks?
Comment 102•13 years ago
|
||
(In reply to comment #98) > That works fine, because you're moving a ghosted drag image of the icon, not > the icon itself. We move a ghosted tab drag image right now in Firefox 3.6. > What we want to do is move the tab itself. I don't see the problem. When moving the tab in the tab bar just move the tab. When moving it out of the tab bar convert it to a ghost image and leave the original tab in the bar. When entering a tab bar again remove the original tab and insert the tab you're dragging at the new location and drag it there.
Comment 103•13 years ago
|
||
+1 at comment #95. This and the "tabs on the title bar" are the two major features I expect and want for FF4. FF4 is already way behind other modern browsers in terms of UI, so I won't be happy if this gets pushed to the next release..
Comment 105•13 years ago
|
||
So can this bug get some attention now that Firefox 4's been released?
Comment 106•13 years ago
|
||
I am using this patch for a few weeks now. I really like this approach to reorder tabs, but I noticed a few flaws: 1.) You can accidentally trigger the tear off. It is quite clear when you tear off a tab vertically (it disappears form the tab bar). But if you reorganize tabs and you let a tab go before it is perfectly in its new position you trigger a tear off. IMHO it would be better if it snaps to the indicated new position. 2.) If you've torn off a tab you have no indication what so ever that the tab is where your mouse pointer is. In the fx4 release there is a small picture of the tab. I think with this new animation the tab should stick to the pointer. 3.) When reattaching a tab, there is only a small arrow indicator. I think it would be nice if this activates the animation, too.
Comment 107•13 years ago
|
||
(In reply to comment #106) Thanks for your feedback. On what platform are you running? Also, this patch is still an incomplete work-in-progress, and I'll be updating it within the week.
Updated•13 years ago
|
Attachment #497424 -
Attachment description: patch v2 (minor and typo fixes) → WIP v2 (minor and typo fixes)
Comment 108•13 years ago
|
||
I’m using Win 7 Professional 64bit. Thanks for all your work, I am looking forward to the updated patch.
Comment 109•13 years ago
|
||
Unbitrotted and updated to use the latest patch from bug 533460 ( https://bug533460.bugzilla.mozilla.org/attachment.cgi?id=520652 ) Pushed to tryserver: http://tbpl.mozilla.org/?tree=MozillaTry&pusher=fyan@mozilla.com&rev=2886cf2446e0 Will test on Windows and Linux when builds are available.
Attachment #497424 -
Attachment is obsolete: true
Updated•13 years ago
|
Whiteboard: [parity-safari][parity-chrome][parity-ie][target-betaN] → [parity-safari][parity-chrome][parity-ie]
Comment 110•13 years ago
|
||
(In reply to comment #109) > Created attachment 524959 [details] [diff] [review] > WIP v3 Works very nice. But dragging tab to bookmarks toolbar doesn't bookmark them.
Comment 111•13 years ago
|
||
(In reply to comment #110) > (In reply to comment #109) > > Created attachment 524959 [details] [diff] [review] > > WIP v3 > Works very nice. But dragging tab to bookmarks toolbar doesn't bookmark them. as previously discussed this will be the wanted behavior, once tabs become more like "window" entities, doesn't make sense to add a window to a toolbar. You can still drag the favicon from the locationbar, usually you will drag the tab for the current visible page, so that should not be any harder.
Comment 112•13 years ago
|
||
In this day and age where touch-screen interfaces will soon be the norm I can see this making a return at the some point in the future. But if it's a trade-off between a modern day feel to tab rearrangement along the tab bar and bookmarking functionality, I'll happily retrain myself to use the star more. Hopefully functionality will be improved in that regards. Bug 582907 deals with that, though not to a degree that it would block this bug.
Comment 113•13 years ago
|
||
Yes, we should probably tweak the star button functionality, I discussed some change with Faaborg at the last all-hands about that, no schedule for now, but I feel like this bug has higher priority than a tab-toolbar drag gesture today.
Comment 114•13 years ago
|
||
Just played with the latest try build. Love the tear-off, and it feels really well done and nippy. I do have a couple points/questions to raise however: 1/ If you tear off a tab and hover of the bookmarks menu (on the bookmarks bar) should it open the bookmarks menu? I would think that the intended behaviour would be to ignore the menu since you can't drop contents there. 2/ Upon dragging a tab to App Tabs it turns into a drop marker. Is this intended? It seems inconsistent? Especially since you can't create App Tabs if you have none placed in this manner. Also if we were going to take this approach, wouldn't it be best to simply shrink the tab upon reaching the App Tab zone? 3/ The styling of a tab when it turns into a Window seems out of place, would it for example be possible to create a glass holder on Windows 7? 4/ If you have a window full of tabs and then tear off one of those tabs, you have a window with a single tab. At that point there tear off threshold disappears, is this correct? However the issue is that the window stays in place, shouldn't it disappear as you have it in your hand/cursor. On a Window full of tabs, the tab disappears from the tab bar the second you start to tear it for example. It seems to me the current implementation requires the user to hover over the icon to get back to the old window and drop the tab there. That seems overly complex for some users.
Comment 115•13 years ago
|
||
(In reply to comment #114) > Just played with the latest try build. Love the tear-off, and it feels really > well done and nippy. > > I do have a couple points/questions to raise however: > 1/ If you tear off a tab and hover of the bookmarks menu (on the bookmarks bar) > should it open the bookmarks menu? I would think that the intended behaviour > would be to ignore the menu since you can't drop contents there. Sounds like a bug. This is handled by http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js, it should actually check if the datatransfer has acceptable data type before opening the menu, looks like it is not doing that.
Comment 116•13 years ago
|
||
better link: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#849
Comment 117•13 years ago
|
||
Dragging is awesome, but the detaching / attaching back is little bit buggy (flickerings and the other tabs don't slide when detaching but jump... ) and too primitive. The prototype (attachment 463283 [details]) would be nice for detaching.
Comment 118•13 years ago
|
||
(In reply to comment #114) > 2/ Upon dragging a tab to App Tabs it turns into a drop marker. Is this > intended? It seems inconsistent? Especially since you can't create App Tabs if > you have none placed in this manner. Also if we were going to take this > approach, wouldn't it be best to simply shrink the tab upon reaching the App > Tab zone? I'd prefer the way it is. > 3/ The styling of a tab when it turns into a Window seems out of place, would > it for example be possible to create a glass holder on Windows 7? Can be done in a follow up.
Comment 119•13 years ago
|
||
How to try this on Windows ?
Comment 120•13 years ago
|
||
You need to install the try-server build. You can find it here : [url]http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/[/url] I agree with [url=https://bugzilla.mozilla.org/show_bug.cgi?id=455694#c117]Comment 117[/url] detaching should be improved one way or another. Otherwise it seems pretty good overall.
Comment 121•13 years ago
|
||
Sorry screwed up, not used to posting here. http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/
Comment 122•13 years ago
|
||
I applaud the UI team for working on this somewhat complex new feature so that the Firefox browser can become more modern and be in line with IE, Safari and Chrome (which already has this feature). This requires a lot of code and polishing to iron out all issues / bugs. However, this feature has been suggested 2.5 years ago and still isn't quite ready yet. Does anyone have an estimated target date for these features? It was supposed to be added to Firefox 4, but what is the time frame now? Firefox 5, 6.....even later than that? I don't see any target / blocking numbers above.
Comment 123•13 years ago
|
||
No earlier than Fx6. The code for Fx5 was already merged over to Aurora, so that ship has sailed.
Comment 124•13 years ago
|
||
(In reply to comment #114) > 1/ If you tear off a tab and hover of the bookmarks menu (on the bookmarks bar) > should it open the bookmarks menu? I would think that the intended behaviour > would be to ignore the menu since you can't drop contents there. (In reply to comment #116) > better link: > http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#849 I had been trying to figure out how to fix that. Thanks for the pointer, Marco. :) (In reply to comment #114) > 2/ Upon dragging a tab to App Tabs it turns into a drop marker. Is this > intended? It is intended for now, simply because fixing that is difficult, and I'd rather get this landed first. It will be handled in followup bug 579629. > 3/ The styling of a tab when it turns into a Window seems out of place, would > it for example be possible to create a glass holder on Windows 7? I'm aware that the styling of the tab is very basic at the moment. Again, this can be done in a followup. > 4/ If you have a window full of tabs and then tear off one of those tabs, you > have a window with a single tab. At that point there tear off threshold > disappears, is this correct? However the issue is that the window stays in > place, shouldn't it disappear as you have it in your hand/cursor. No. One of the key points is that it should be easy to reattach during the drag. The threshold and detach animation could definitely be tweaked, but I'll leave that detail to a followup. (In reply to comment #117) > Dragging is awesome, but the detaching / attaching back is little bit buggy > (flickerings and the other tabs don't slide when detaching but jump... ) and > too primitive. The prototype (attachment 463283 [details]) would be nice for detaching. I am aware. (In reply to comment #120) > You need to install the try-server build. > http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/ Yup, that's the best build to try it so far. I haven't been advertising the tryserver builds just yet, because I haven't done enough testing on Windows and Linux. I estimate that by Monday, the patch will be ready for review, and I'll link a new tryserver build by then.
Comment 125•13 years ago
|
||
> (In reply to comment #116) > > better link: > > http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#849 > > I had been trying to figure out how to fix that. Well, the tricky part is that you should walk all the dataTransfer (since I could be dragging multiple elements) and see if some of the data is not accepted (iirc if even just 1 data is not accepted we don't allow the drop). The good news is that probably you can use PlacesControllerDragHelper.canDrop (something similar to what we do here http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#914) This code is still a bit hairy, so in case feel free to drop the issue to a follow-up, it should not block the feature from landing.
Comment 126•13 years ago
|
||
(In reply to comment #124) > (In reply to comment #114) > (In reply to comment #117) > > Dragging is awesome, but the detaching / attaching back is little bit buggy > > (flickerings and the other tabs don't slide when detaching but jump... ) and > > too primitive. The prototype (attachment 463283 [details]) would be nice for detaching. > > I am aware. No offense intended. Are you planning to add some behaviour like in the prototype?
Updated•13 years ago
|
Attachment #496124 -
Attachment description: patch v1 → WIP v4
Updated•13 years ago
|
Attachment #497424 -
Attachment description: WIP v2 (minor and typo fixes) → WIP v5
Updated•13 years ago
|
Attachment #524959 -
Attachment description: WIP v3 → WIP v6
Comment 127•13 years ago
|
||
The patch is ready for review now, AFAICT. Windows and OS X users can try out these builds: https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/ The patch's code has been slightly modified since that build was made, but the behavior didn't really change, IIRC. Stuff is still broken on Linux, but that's due to the patch for bug 533460 being incomplete.
Attachment #524959 -
Attachment is obsolete: true
Attachment #526442 -
Flags: ui-review?(limi)
Attachment #526442 -
Flags: review?(dolske)
Comment 128•13 years ago
|
||
As I said, the drag panel that appears when tabs are detached still doesn't seem to work on Linux, so we might need this #ifdef workaround for Linux, if we can't fix it in time. LINUX, Y U NO WORK :(
Comment 129•13 years ago
|
||
What doesn't work about it in Linux?
Comment 130•13 years ago
|
||
Could be this is related to the longstanding drag bug Places has on Linux? For us, in Linux, as soon as a popup opens, it's impossible to start a drag (for example folders in the toolbar, they work because we try to figure out the will of the user based on previous mouse movements).
Comment 131•13 years ago
|
||
(In reply to comment #129) > What doesn't work about it in Linux? Using this build: https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/tryserver-linux/ When I start dragging a tab, the drag panel appears very briefly and then disappears, which also internally cancels the drag operation, so I cannot move the tab. The expected behavior (and actual behavior on Windows and OS X) is that the panel is open but invisible due to opacity:0 until dragging such that the tab would be detached at which point opacity:1 is set so that the panel is visible. (In reply to comment #130) > Could be this is related to the longstanding drag bug Places has on Linux? > For us, in Linux, as soon as a popup opens, it's impossible to start a drag > (for example folders in the toolbar, they work because we try to figure out the > will of the user based on previous mouse movements). That might be it...
Comment 132•13 years ago
|
||
Actually, even with the #ifdef workaround, the drag image still disappears like that, with no errors in the Error Console as evidence. Now I'm not sure what the problem is. x(
Comment 133•13 years ago
|
||
(In reply to comment #132) > Actually, even with the #ifdef workaround, the drag image still disappears like > that, with no errors in the Error Console as evidence. Now I'm not sure what > the problem is. x( Oops, I was using XP_LINUX, which isn't defined in that file. This fixed patch addendum demonstrates that the panel is not working and actually breaking the drag on Linux.
Attachment #526446 -
Attachment is obsolete: true
Comment 134•13 years ago
|
||
To clarify, the above addendum resets the drag image to a blank canvas, instead of using the panel, since my attempts to use the panel have so far only resulted in the drag operation being aborted immediately after dragstart.
Comment 135•13 years ago
|
||
I have tried the patch and it works nice, great work! There is one thing that I think should be solve: when you drag the last tab of a window, the window should dissapear during the dragging process, allowing you to drag its tab to a windows which is just behind.
Comment 136•13 years ago
|
||
yeah , just new tab button doesnt look good and is illogical too
Comment 137•13 years ago
|
||
(In reply to comment #135) > I have tried the patch and it works nice, great work! There is one thing that I > think should be solve: when you drag the last tab of a window, the window > should dissapear during the dragging process, allowing you to drag its tab to a > windows which is just behind. Chrome 12 already does this. This should be implemented for aero snap (Bug 563540) as well.
Comment 138•13 years ago
|
||
(In reply to comment #135) (In reply to comment #136) (In reply to comment #137) Patches are welcome. (In reply to comment #125) > Well, the tricky part is that you should walk all the dataTransfer (since I > could be dragging multiple elements) and see if some of the data is not > accepted ... > This code is still a bit hairy, so in case feel free to drop the issue to a > follow-up, it should not block the feature from landing. Thanks for the pointers. I think this patch already has enough complexity. I'll leave that a followup, which perhaps you would be better suited to take. ;)
Comment 139•13 years ago
|
||
(In reply to comment #135) > I have tried the patch and it works nice, great work! There is one thing that I > think should be solve: when you drag the last tab of a window, the window > should dissapear during the dragging process, allowing you to drag its tab to a > windows which is just behind. see Bug 597989
Comment 140•13 years ago
|
||
(In reply to comment #131) > (In reply to comment #129) > > What doesn't work about it in Linux? > > Using this build: > https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/tryserver-linux/ > > When I start dragging a tab, the drag panel appears very briefly and then > disappears, which also internally cancels the drag operation, so I cannot move > the tab. I investigated the problem further, and it seems that immediately after a property is set that would cause the panel to be rendered (e.g. panel.hidden = false;), the above happens (appear, disappear, drag cancelled). I haven't been able to find a solution or workaround besides using a blank canvas instead of the panel on Linux for now, which I think is acceptable, since Linux has a special cursor for dragging, which mitigates the detach feedback problem.
Comment 141•13 years ago
|
||
Attachment #528244 -
Flags: review?(sdwilsh)
Comment 142•13 years ago
|
||
Optimized tab positioning code. Polished dragging between app tab and normal tab regions.
Attachment #526442 -
Attachment is obsolete: true
Attachment #528246 -
Flags: ui-review?(limi)
Attachment #528246 -
Flags: review?(dolske)
Attachment #526442 -
Flags: ui-review?(limi)
Attachment #526442 -
Flags: review?(dolske)
Comment 143•13 years ago
|
||
Attachment #526467 -
Attachment is obsolete: true
Attachment #528247 -
Flags: review?(dolske)
Comment 144•13 years ago
|
||
Comment on attachment 528244 [details] [diff] [review] Places changes >diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js > // Thus we shouldn't use unwrapNodes here at all if possible. > // I think it would be OK to accept bogus data here (e.g. text which was > // somehow wrapped as TAB_DROP_TYPE, this is not in our control, and > // will just case the actual drop to be a no-op), and only rule out valid > // expected cases, which are either unsupported flavors, or items which > // cannot be dropped in the current insertionpoint. The last case will > // likely force us to use unwrapNodes for the private data types of > // places. >- if (flavor == TAB_DROP_TYPE) >- continue; >- you didn't fix the above comment that talks about TAB_DROP_TYPE. also, what discussed in comment 115 appears not addressed (nor a follow-up has been filed).
Attachment #528244 -
Flags: review?(sdwilsh) → review-
Comment 145•13 years ago
|
||
(In reply to comment #144) Thanks for the review. :) > you didn't fix the above comment that talks about TAB_DROP_TYPE. Fixed. > also, what discussed in comment 115 appears not addressed (nor a follow-up has > been filed). Addressed and fixed, AFAICT.
Attachment #528244 -
Attachment is obsolete: true
Attachment #528455 -
Flags: review?(mak77)
Comment 146•13 years ago
|
||
Comment on attachment 528455 [details] [diff] [review] Places changes v2 Review of attachment 528455 [details] [diff] [review]: r=me with these changes, assuming you manually tested that dragging a tab does not open the menu, while dragging a bookmark from the toolbar and the favicon from the locationbar still open the menu correctly. ::: browser/base/content/browser-places.js @@ +850,5 @@ // Opening menus in a Places popup is handled by the view itself. if (!this._isStaticContainer(event.target)) return; + if (PlacesControllerDragHelper.canDrop(null, event.dataTransfer)) { Here you should generate an insertion point to the end of the bookmarks menu, let ip = new InsertionPoint(PlacesUtils.bookmarksMenuFolderId, PlacesUtils.bookmarks.DEFAULT_INDEX, Ci.nsITreeView.DROP_ON); and pass it as the first argument to canDrop ::: browser/components/places/content/controller.js @@ +1369,5 @@ return false; } + if (!ip) // We might not have an insertion point yet (e.g. onDragEnter). + continue; I suppose you did this because you were passing a null ip, I prefer if you pass a guessed ip to the bookmarks menu, as said above, and avoid this change.
Attachment #528455 -
Flags: review?(mak77) → review+
Comment 147•13 years ago
|
||
(In reply to comment #146) > r=me with these changes, assuming you manually tested that dragging a tab does > not open the menu, while dragging a bookmark from the toolbar and the favicon > from the locationbar still open the menu correctly. Yes, I tested it :) > PlacesUtils.bookmarks.DEFAULT_INDEX, I misunderstood what DEFAULT_INDEX meant, so I thought it couldn't be used, but I get it now. Thanks for the review!
Attachment #528455 -
Attachment is obsolete: true
Comment 148•13 years ago
|
||
Try builds expire, so here's a (semi)permalink to builds with this patch (and some other tab-related patches): https://people.mozilla.com/~fyan/try-builds/tab-animations/
Comment 149•13 years ago
|
||
I think that Moving Animation is too slow while you move tabs fast. It looks like kind of "animation freeze" for a few first moments of animation.
Comment 150•13 years ago
|
||
I stumbled across a problem: If a loading tab triggers the master password dialog while you are dragging a tab, the whole window freezes, nothing is clickable and you eventually have to kill the firefox process.
Comment 151•13 years ago
|
||
I use a single core processor Intel Celeron. When you try to move the tabs, the CPU loaded at 100%. What causes it impossible to freezes. I want to note that a similar animation in Chrome, Opera or IE working smoothly and seamlessly.
Comment 152•13 years ago
|
||
I found something : You could call it a bug or a missing feature. When we have 2 windows of firefox open. If we detach one tab from one of the window(say window 1) and drag it to the tab strip of the other window(say windows 2), the tab from the tab strip of window 1 is removed , but no tab is added to the tab strip of window 2. Just the arrow is shown. What is required would be something like this : we detach a tab from window 1 , the tab from the tab strip of Window 1 immediately disappears. And when we add that tab to Window 2, its tab should be added to Window 2's tab strip along with the tab rearrangement animation in action. I hope my idea is clear to all and I wish to see this feature in this animation.
Updated•13 years ago
|
Comment 153•13 years ago
|
||
(In reply to comment #148) > Try builds expire, so here's a (semi)permalink to builds with this patch > (and some other tab-related patches): > > https://people.mozilla.com/~fyan/try-builds/tab-animations/ Good job Frank! Is in intended that there is no animation when moving normal tabs into app tabs area?
blocking2.0: - → ?
Updated•13 years ago
|
blocking2.0: ? → ---
Comment 154•13 years ago
|
||
Comment on attachment 528246 [details] [diff] [review] patch v2 Review of attachment 528246 [details] [diff] [review]: ----------------------------------------------------------------- This is basically r+, but I'm just going to clear review instead, since there are a bunch of nits and a few questions. Will r+ an updated version, assuming all else turns out well! \o/ ::: browser/base/content/browser.css @@ +80,5 @@ > + > +.tabbrowser-tabs[drag=move] > .tabbrowser-tab:not([selected]) { > + -moz-transition: max-width 1ms, -moz-transform 200ms ease-out; > +} > + So, AIUI (from talking with Frank), this 1ms transition on max-width is just to ensure we always get a transitionend event -- specifically, when a tab closes while dragging it (since we currently hook transitionend to finish tab-close stuff). It's otherwise irrelevant to the feature this patch is implementing. I really hate how CSS transitions work in this respect... Since CSS only applies 1 rule, you can't have independent implementations of things using -moz-transition for the same node. [Just as "color: black" and "color: white" doesn't give you grey. But this is more akin to deconflicting "border: X" and "border: Y" by having separate "border-color: X" and "border-style: Y" properties.] So, definitely add a comment here, and we should talk with dbaron if there's something that can be done with the spec to help in the long run. ::: browser/base/content/tabbrowser.xml @@ +1663,5 @@ > tabs._scrolledToEnd = false; > }, 0, this.tabContainer); > } > + > + this.tabContainer._finishTabDrag(aTab); Just FYI, this chunk no longer applies cleanly on trunk. @@ +2660,5 @@ > > <content> > <xul:hbox align="end"> > + <xul:panel type="drag-image" allowevents="false" hidden="true" > + class="tab-drag-image" anonid="tab-drag-image"/> Dunno if it's specifically from this, but on my Windows debug build, when I drag the foreground tab I get an assert in nsMenuPopupFrame.cpp:1147 about mPrefSize.width/.height not expected to be zero. It's also super slow and janky (like 1Hz update rate, .gifs freeze up too) while dragging. Not sure what's up with that, something about my debug build, I assume. Hmm, well, my OS X debug build is nice and smooth, though. Have you tried this on a Windows debug build? @@ +2756,5 @@ > <field name="_tabDropIndicator"> > document.getAnonymousElementByAttribute(this, "anonid", "tab-drop-indicator"); > </field> > > + <method name="_positionDropIndicator"> As noted when we talked, this function is just a move of the old |dragover| handler, with the trivial addition of the |scrollOnly| arg. @@ +2853,5 @@ > + ]]></body> > + </method> > + > + <field name="_tabDragImage"> > + document.getAnonymousElementByAttribute(this, "anonid", "tab-drag-image"); Sigh. I wonder how much time we spend evaluating all the <field>s in this file... @@ +2861,5 @@ > + <parameter name="event"/> > + <body><![CDATA[ > + let draggedTab = event.dataTransfer.mozGetDataAt(TAB_DROP_TYPE, 0); > + if (!draggedTab || !draggedTab.parentNode) > + return; The .parentNode check is just here to make sure the tab hasn't been closed, right? Maybe add a <property name="isStillOpen" onget="this.parentNode != null"/>? Self-documenting code FTW. :) @@ +2864,5 @@ > + if (!draggedTab || !draggedTab.parentNode) > + return; > + > + let parent = draggedTab.parentNode; > + if (parent.childNodes.length == 1) { parent.childElementCount == 1? @@ +2872,5 @@ > + > + let bo = this.mTabstrip.boxObject; > + // detach if leaving window or at least a tab height down from tabbar > + // Note: we do not have the ability to detect drags over the titlebar > + if (event.type == "dragexit" || event.screenY > bo.screenY + 2 * bo.height) { For future's sake, please expand this comment a tiny bit wrt why the tab height bit is done. @@ +2881,5 @@ > + delete draggedTab._dropIndex; > + } > + // TODO: we should call _blurTab here, but it won't be safe until > + // Panorama is properly integrated and behaves nicely > + return true; Please file a bug for this and note it here. @@ +2888,5 @@ > + parent.setAttribute("drag", "move"); > + ]]></body> > + </method> > + > + <method name="_handleTabDrag"> _handleTabDragover() would be more accurate. @@ +2894,5 @@ > + <body><![CDATA[ > + let draggedTab = event.dataTransfer.mozGetDataAt(TAB_DROP_TYPE, 0); > + if (!draggedTab.parentNode) // tab was closed during drag > + return; > + event.preventDefault(); ... // indicate a drop is possible @@ +2895,5 @@ > + let draggedTab = event.dataTransfer.mozGetDataAt(TAB_DROP_TYPE, 0); > + if (!draggedTab.parentNode) // tab was closed during drag > + return; > + event.preventDefault(); > + event.stopPropagation(); Is the .stopPropagation really needed here? @@ +2901,5 @@ > + return; > + > + let newIndex = this._getDropIndex(event, draggedTab); > + let tabAtNewIndex = this.childNodes[newIndex > draggedTab._tPos ? > + newIndex-1 : newIndex]; |newIndex - 1| (spacing) @@ +2942,5 @@ > + else if (tab._tPos > draggedTab._tPos && tab._tPos < newIndex) > + tab.style.MozTransform = "translate(" + -tabWidth + "px)"; > + else > + tab.style.MozTransform = ""; > + }); Couple of comments here: 1) Part of me wonders if it would be better to avoid the -moz-transforms, and instead insert dummy/invisible tab(s), plus controlling their width, in order to make the bits slide around. But I don't see anything fundamentally _wrong_ with the transform, and that would mean rewriting this from scratch. So, fine. :) 2) While playing with this I noticed a bit of glitchyness on OS X... While dragging a tab, sometimes the other tabs would get a :hover hilight applied, and then that other tab flickers back to the non-:hover state when you drop the tab. I think this can be addressed in a followup if you file it, though. :) @@ +3226,5 @@ > + case "dragover": > + this._handleTabDrag(aEvent); > + break; > + case "dragexit": > + if (aEvent.target == document) Why check |.target == document|? Sidenote: our documentation sure is vague about the difference/existence of dragexit vs dragleave. @@ +3303,5 @@ > + let tabs = this.tabbrowser.visibleTabs; > + > + if (draggedTab) { > + let tabX = draggedTab.boxObject.screenX + draggedTab._dragDistX; > + let right = draggedTab._dragDistX > 0; s/right/[dragging|moving]Right/? @@ +3313,5 @@ > + if (compare(eX, tabs[i].boxObject.screenX + tabs[i].boxObject.width / 2, ltr)) > + return i; > + > + let i = tabs.indexOf(draggedTab), tab = draggedTab, next; > + while (next = ltr ^ right ? tabs[--i] : tabs[++i]) { Congrats on a rare, legit use of XOR. :) @@ +3324,5 @@ > + x < this.mTabstrip._scrollButtonUp.boxObject.screenX) > + break; > + tab = next; > + } > + return tab._tPos + (ltr ^ right ? 0 : 1); ...again. :) @@ +3330,5 @@ > + > + let tab = this._getDragTargetTab(event); > + for (let i = tab ? tab._tPos : 0; i < tabs.length; i++) > + if (compare(eX, tabs[i].boxObject.screenX + tabs[i].boxObject.width / 2, ltr)) > + return tabs[i]._tPos; This loop is subtly different now, in that |tabs| is .visibleTabs and not .childNodes, but that looks to be ok. @@ +3335,5 @@ > + return this.childNodes.length; > + ]]></body> > + </method> > + > + <method name="_moveTab"> This method name is a little confusing, given there's already a _moveTabTo(). What was it we came up with? slideTab()? @@ +3369,5 @@ > + if (window.getComputedStyle(this).direction == "rtl") > + displacement *= -1; > + let transform = "translate(" + displacement + "px)"; > + let currentTransform = draggedTab.style.MozTransform; > + if (currentTransform && currentTransform != transform) { Add comment on this "caching". @@ +3377,5 @@ > + if (event.propertyName != "-moz-transform") > + return; > + this.removeEventListener("transitionend", finish, false); > + this.parentNode._finishTabDrag(this); > + this.parentNode.tabbrowser.moveTabTo(this, newIndex); Hmm. I suppose this is one of the seemingly few times where an event listener's |this| is a useful value. Still, I'd posit that it would be worth making this a bit more robust by checking event.eventPhase == Event.AT_TARGET, lest this code ever see a bubbling transitionend from elsewhere. And maybe s/this/draggedTab/, just to avoid the "hey, it's using this|, wonder if it's intentional". @@ +3563,5 @@ > + if (this.childNodes.length == 1) { > + let windows = Services.wm.getEnumerator("navigator:browser"), n = 0; > + for (; windows.hasMoreElements(); windows.getNext(), n++); > + if (n == 1) // only one tab and one browser window, so nowhere to drop > + return; What if there's another window, but it's a popup window? [ala nsIBrowserGlue's getMostRecentBrowserWindow()]. I suppose there's no point in making this code more complex (and potentially confusing to the user?) for such an edge case, though. @@ +3574,5 @@ > + > + let dragImage = this._tabDragImage; > + dragImage.appendChild(tabPreviews.get(tab)); > + dragImage.hidden = false; > + dt.setDragImage(dragImage, -1, -1); I'm a little confused here. Why are you doing .setDragImage() here, when the panel is also being unhidden? @@ +3596,5 @@ > + ]]></handler> > + > + <handler event="drop"><![CDATA[ > + this._tabDropIndicator.collapsed = true; > + event.stopPropagation(); Again with the .stopPropagation()? Oh, I guess this is just moving code. Hmm. @@ +3610,5 @@ > + if (draggedTab) { > + let parent = draggedTab.parentNode; > + if (parent == this) { > + // It is better to handle tab moving on drop when possible, > + // since there is a lag between the drop and dragend events. Did you investigate and/or file a bug on the apparent delay? @@ +3636,5 @@ > > // We need to select the tab after we've done > // swapBrowsersAndCloseOther, so that the updateCurrentBrowser > // it triggers will correctly update our URL bar. > + this.selectedItem = newTab; Why this change?
Attachment #528246 -
Flags: ui-review?(limi)
Attachment #528246 -
Flags: review?(dolske)
Comment 155•13 years ago
|
||
(In reply to comment #154) > @@ +3636,5 @@ > > > > // We need to select the tab after we've done > > // swapBrowsersAndCloseOther, so that the updateCurrentBrowser > > // it triggers will correctly update our URL bar. > > + this.selectedItem = newTab; > > Why this change? Boo splinter. This comment makes more sense when you view it as the original diff: - this.tabbrowser.selectedTab = newTab; + this.selectedItem = newTab;
Comment 156•13 years ago
|
||
(In reply to comment #154) > > +.tabbrowser-tabs[drag=move] > .tabbrowser-tab:not([selected]) { > > + -moz-transition: max-width 1ms, -moz-transform 200ms ease-out; > > +} > > + > I really hate how CSS transitions work in this respect... Since CSS only > applies 1 rule, you can't have independent implementations of things using > -moz-transition for the same node. [Just as "color: black" and "color: > white" doesn't give you grey. But this is more akin to deconflicting > "border: X" and "border: Y" by having separate "border-color: X" and > "border-style: Y" properties.] I concur. There are improvements being proposed to the CSSOM that will hopefully solve this problem. > So, definitely add a comment here, and we should talk with dbaron if there's > something that can be done with the spec to help in the long run. Will do. > > <content> > > <xul:hbox align="end"> > > + <xul:panel type="drag-image" allowevents="false" hidden="true" > > + class="tab-drag-image" anonid="tab-drag-image"/> > > Dunno if it's specifically from this, but on my Windows debug build, when I > drag the foreground tab I get an assert in nsMenuPopupFrame.cpp:1147 about > mPrefSize.width/.height not expected to be zero. > > It's also super slow and janky (like 1Hz update rate, .gifs freeze up too) > while dragging. Not sure what's up with that, something about my debug > build, I assume. Hmm, well, my OS X debug build is nice and smooth, though. > Have you tried this on a Windows debug build? No, that's odd. I don't have a Windows machine with me (while working remotely), so debugging that may have to wait another week. > Sigh. I wonder how much time we spend evaluating all the <field>s in this > file... I'll make a note in my todo list to benchmark this sometime. > > + if (!draggedTab || !draggedTab.parentNode) > > + return; > > The .parentNode check is just here to make sure the tab hasn't been closed, > right? > > Maybe add a <property name="isStillOpen" onget="this.parentNode != null"/>? > > Self-documenting code FTW. :) Okay. The binding still works even after it's removed from the document? That's interesting. > > + if (parent.childNodes.length == 1) { > > parent.childElementCount == 1? Ooh, right! :) > > + // detach if leaving window or at least a tab height down from tabbar > > + // Note: we do not have the ability to detect drags over the titlebar > > For future's sake, please expand this comment a tiny bit wrt why the tab > height bit is done. Will do. > > + // TODO: we should call _blurTab here, but it won't be safe until > > + // Panorama is properly integrated and behaves nicely > > Please file a bug for this and note it here. Bug 657097 filed. Will be noted. > > + <method name="_handleTabDrag"> > > _handleTabDragover() would be more accurate. Agreed. > > + let draggedTab = event.dataTransfer.mozGetDataAt(TAB_DROP_TYPE, 0); > > + if (!draggedTab.parentNode) // tab was closed during drag > > + return; > > + event.preventDefault(); > > + event.stopPropagation(); > > Is the .stopPropagation really needed here? IIRC, yes. It prevents the <handler event="dragover"> code from running. > 1) Part of me wonders if it would be better to avoid the -moz-transforms, > and instead insert dummy/invisible tab(s), plus controlling their width, in > order to make the bits slide around. But I don't see anything fundamentally > _wrong_ with the transform, and that would mean rewriting this from scratch. > So, fine. :) I pondered this too, but I found other techniques that I considered (e.g. inserting temporary DOM elements) to have even more risk of creating inconsistent, broken states, especially if a race condition lurked or an unanticipated user action was made. > 2) While playing with this I noticed a bit of glitchyness on OS X... While > dragging a tab, sometimes the other tabs would get a :hover hilight applied, > and then that other tab flickers back to the non-:hover state when you drop > the tab. I think this can be addressed in a followup if you file it, though. > :) I noticed this too early on. I'm not sure if this is my fault or a bug in the :hover calculation. This is possibly related to another glitch that I recently discovered with this patch: it seems to regress bug 465086 randomly. I'd appreciate help in investigating these, since I've made little progress in identifying a cause. I'll file a bug once we at least fix/identify the bug 465086 regression. > > + case "dragexit": > > + if (aEvent.target == document) > > Why check |.target == document|? The only time we're sure we want to set the tab to detach state is when the drag is leaving the document. Otherwise, it could be leaving any random child node. > Sidenote: our documentation sure is vague about the difference/existence of > dragexit vs dragleave. AFAIK, dragexit is an event that we've had in XUL for a long time. https://developer.mozilla.org/en/XUL/Events Windows Form APIs had a dragleave event, so Microsoft brought that to IE. When it came to standarding the drag events, the working group folks adopted the IE events, and so we got dragleave. We fired dragexit events before dragenter events as is sensible. However, IE fired dragenter events before dragleave events, and the spec adopted their ordering, so we made our code conform too ( bug 527749 ). Thus, the order of events fired in Gecko is: dragexit, dragenter, dragleave. > > + let right = draggedTab._dragDistX > 0; > > s/right/[dragging|moving]Right/? Ah yes. This was a case of quickly named variable that never got properly renamed. > > + while (next = ltr ^ right ? tabs[--i] : tabs[++i]) { > > Congrats on a rare, legit use of XOR. :) > > + return tab._tPos + (ltr ^ right ? 0 : 1); > > ...again. :) \o/ I relish these operator usage opportunities! > > + <method name="_moveTab"> > > This method name is a little confusing, given there's already a > _moveTabTo(). What was it we came up with? slideTab()? Yeah, _slideTab(). It doesn't slide the tab if the pinned state will change, but once we fix bug 579629, it will. > > + let currentTransform = draggedTab.style.MozTransform; > > + if (currentTransform && currentTransform != transform) { > > Add comment on this "caching". It's not caching. If the transform already matches, then no transition will be made, so no transitionend will be called, and the code will fail! Hence the check. Nevertheless, you're absolutely right that it should have a comment! :) > > + this.removeEventListener("transitionend", finish, false); > > + this.parentNode._finishTabDrag(this); > > + this.parentNode.tabbrowser.moveTabTo(this, newIndex); > > Hmm. I suppose this is one of the seemingly few times where an event > listener's |this| is a useful value. > > Still, I'd posit that it would be worth making this a bit more robust by > checking event.eventPhase == Event.AT_TARGET, lest this code ever see a > bubbling transitionend from elsewhere. And maybe s/this/draggedTab/, just to > avoid the "hey, it's using > this|, wonder if it's intentional". Good idea. I used |this|, because I thought I could avoid pulling things from the closure, but I ended up having to do it anyway. > > + if (this.childNodes.length == 1) { > > + let windows = Services.wm.getEnumerator("navigator:browser"), n = 0; > > + for (; windows.hasMoreElements(); windows.getNext(), n++); > > + if (n == 1) // only one tab and one browser window, so nowhere to drop > > + return; > > What if there's another window, but it's a popup window? [ala > nsIBrowserGlue's getMostRecentBrowserWindow()]. > > I suppose there's no point in making this code more complex (and potentially > confusing to the user?) for such an edge case, though. Is there a quick way to check if a window is a popup window? > > + let dragImage = this._tabDragImage; > > + dragImage.appendChild(tabPreviews.get(tab)); > > + dragImage.hidden = false; > > + dt.setDragImage(dragImage, -1, -1); > > I'm a little confused here. Why are you doing .setDragImage() here, when the > panel is also being unhidden? If I don't call .setDragImage, the default drag image will be the node I'm dragging, so I need to set drag image, in this case, to the <panel>. I keep the <panel> hidden most of the time to avoid it being accidentally rendered or read by screen readers, etc. In other words, setting panel.hidden to false alone does not actually make it appear onscreen nor does it "attach" it to the cursor movements. > > + <handler event="drop"><![CDATA[ > > + this._tabDropIndicator.collapsed = true; > > + event.stopPropagation(); > > Again with the .stopPropagation()? Oh, I guess this is just moving code. Hmm. Yeah, I just moved code. I simply decided not to risk touching it. > @@ +3610,5 @@ > > + if (draggedTab) { > > + let parent = draggedTab.parentNode; > > + if (parent == this) { > > + // It is better to handle tab moving on drop when possible, > > + // since there is a lag between the drop and dragend events. > > Did you investigate and/or file a bug on the apparent delay? I investigated it on OS X, and it seems that the delay does not exist; I'll confirm on Windows and Linux asap. (Well, there is a delay, but it's ~1ms (tested with both a simple HTML testcase and tabbrowser.xml), so I'm not worried about it.) I guess it was a bug with an old version of my patch or something. Consequently, I removed the call to _slideTab() from the drop handler. Since now the only caller of _slideTab() is the "dragend" handler, should I leave it as a separate function or inline it?
Updated•13 years ago
|
Blocks: animations
tracking-firefox6:
--- → ?
Updated•13 years ago
|
tracking-firefox6:
? → ---
Comment 157•13 years ago
|
||
When it will be added to the nightly?
Comment 158•13 years ago
|
||
when it will be ready
Comment 159•13 years ago
|
||
We never see this in firefox 7?
Comment 160•13 years ago
|
||
Frank Yan, could you please push this feature to UX branch?
Comment 161•13 years ago
|
||
Unbitrotted and addressed review comments. Not ready for review. 1 remaining issue :( Ready for help on Linux when you're available, Neil. Thanks in advance :)
Attachment #528246 -
Attachment is obsolete: true
Comment 162•13 years ago
|
||
Comment 163•13 years ago
|
||
(In reply to comment #162) > Created attachment 540322 [details] > Last tab drag drop changes nav bar height Disable Stratiform, if this still happen, disable all the add-ons and plug-in, use the default theme
Comment 164•13 years ago
|
||
(In reply to comment #162) > Created attachment 540322 [details] > Last tab drag drop changes nav bar height Disable Stratiform, if this still happen, disable all the add-ons and plug-in, use the default theme without any personas.
Comment 165•13 years ago
|
||
Happens again , but with less magnitude , i.e. the height does reduce , but by less px
Comment 166•13 years ago
|
||
Comment on attachment 528247 [details] [diff] [review] addendum to disable detach panel on Linux for now (Clearing old review request for linux workaround. Should be fine if this ends up landing before but 533460 is fixed, though.)
Attachment #528247 -
Flags: review?(dolske)
Comment 167•13 years ago
|
||
This is somewhat a reference back to comment #19, but I've just noticed (in the UX nightly) that attempting to duplicate the tabs (by pressing ctrl and dragging) instead moves the tab into a new window. Expected behavior, of course, would be to create a duplicate tab
Comment 168•13 years ago
|
||
This fixes the regression of bug 465086. The cause was the destruction and recontruction of the tabbrowser-tab binding whenever a tab moved around in the DOM. The solution is to make _fullyOpen a regular JS expando property, not an XBL field. This also includes a partial workaround for bug 666864, which causes the incorrect application of :hover styling to a background tab during and after a drag operation.
Attachment #528247 -
Attachment is obsolete: true
Attachment #539762 -
Attachment is obsolete: true
Attachment #541621 -
Flags: review?(dolske)
Updated•13 years ago
|
Attachment #541621 -
Flags: ui-review?(limi)
Comment 169•13 years ago
|
||
Fixes issue illustrated by https://bug455694.bugzilla.mozilla.org/attachment.cgi?id=540322 Also fixes a typo and a few edge cases.
Attachment #540322 -
Attachment is obsolete: true
Attachment #541621 -
Attachment is obsolete: true
Attachment #542302 -
Flags: ui-review?(limi)
Attachment #542302 -
Flags: review?(dolske)
Attachment #541621 -
Flags: ui-review?(limi)
Attachment #541621 -
Flags: review?(dolske)
Comment 170•13 years ago
|
||
Comment on attachment 542302 [details] [diff] [review] patch v5 We've been running this on the UX branch for the past weeks, and it has worked great so far. Let's land it!
Attachment #542302 -
Flags: ui-review?(limi) → ui-review+
Comment 171•13 years ago
|
||
YAYYY !! Lets do it
Comment 172•13 years ago
|
||
BTW I know its off topic, But Alexander Limi , why was the new tab prototype v1 removed from ux branch ? May be for v2 ?
Comment 173•13 years ago
|
||
(In reply to comment #170) > Comment on attachment 542302 [details] [diff] [review] [review] > patch v5 > > We've been running this on the UX branch for the past weeks, and it has > worked great so far. Let's land it! But it's still laggy when you move tabs fast enough on Windows (7, and I don't know anything about other platforms). Chrome and Opera have no tab animation lag...
Comment 174•13 years ago
|
||
(In reply to comment #173) > But it's still laggy when you move tabs fast enough on Windows (7, and I > don't know anything about other platforms). Chrome and Opera have no tab > animation lag... Confirmed!
Comment 175•13 years ago
|
||
Please do not landed in this form. Get the disappearance of lags, then add.
Comment 176•13 years ago
|
||
Yes , the tab dragging animation is lagging a lot, slow and feels heavy. Chrome's tab dragging animation is smooth and feels so light.
Comment 177•13 years ago
|
||
Not sure which patch was applied on Firefox UX branch but it looks like that this patch broke function "Duplicate tab" (press CTRL and move tab). It works fine on latest trunk.
Comment 178•13 years ago
|
||
Comment on attachment 542302 [details] [diff] [review] patch v5 Random drive-by comments: >--- a/browser/base/content/browser.css >+++ b/browser/base/content/browser.css >@@ -57,16 +57,40 @@ tabbrowser { > -moz-transition: opacity 250ms; > } > > .tabbrowser-tabs[positionpinnedtabs] > .tabbrowser-tab[pinned] { > position: fixed !important; > display: block; /* position:fixed already does this (bug 579776), but let's be explicit */ > } > >+.tabbrowser-tab[selected] { >+ z-index: 2; >+} It's not clear what this is supposed to do, needs a comment. > <method name="addTab"> > <parameter name="aURI"/> >- <parameter name="aReferrerURI"/> >+ <parameter name="aReferrerURI"/>up > <parameter name="aCharset"/> > <parameter name="aPostData"/> > <parameter name="aOwner"/> > <parameter name="aAllowThirdPartyFixup"/> ... > <handler event="underflow" phase="capturing"><![CDATA[ >- if (event.detail == 0) >+ if (event.target != this || event.detail == 0) > return; // Ignore vertical events This looks wrong to me. I think the target is supposed to be the <scrollbox>, not the <arrowscrollbox>. If this works as is, then that's probably due to xbl being involved. In that case, check event.originalTarget. >+ if (event.type == "dragexit" || event.screenY > bo.screenY + 2 * bo.height) { >+ if (parent.getAttribute("drag") != "detach") { >+ parent.setAttribute("drag", "detach"); >+ parent._clearTransforms(); >+ parent._tabDropIndicator.collapsed = true; >+ delete draggedTab._dropIndex; >+ if (parent.tabbrowser.visibleTabs.length == 1) >+ draggedTab.style.maxWidth = ""; >+ } >+ // TODO: we should call _blurTab here, but it won't be safe until >+ // Panorama is properly integrated and behaves nicely Unclear what this means. Why should _blurTab be called and what are the consequences of not doing it? What's the problem with panorama? >+ <method name="_clearTransforms"> Needs a more distinctive name, _dragClearTransforms if nothing else; doesn't need to be super-nice since it's private.
Comment 179•13 years ago
|
||
(In reply to comment #178) > >+.tabbrowser-tab[selected] { > >+ z-index: 2; > >+} > > It's not clear what this is supposed to do, needs a comment. Will add the comment: /* keep selected tab on top during drags */ > > <handler event="underflow" phase="capturing"><![CDATA[ > >- if (event.detail == 0) > >+ if (event.target != this || event.detail == 0) > > return; // Ignore vertical events > > This looks wrong to me. I think the target is supposed to be the > <scrollbox>, not the <arrowscrollbox>. If this works as is, then that's > probably due to xbl being involved. In that case, check event.originalTarget. It works as is, but I'll change it to check event.originalTarget. > >+ // TODO: we should call _blurTab here, but it won't be safe until > >+ // Panorama is properly integrated and behaves nicely > > Unclear what this means. Why should _blurTab be called and what are the > consequences of not doing it? What's the problem with panorama? Ideally, the UI should preview to the user what will happen when the user releases the pointing device. That means, when the user drags the tab beyond the detachment threshold, we should call some form of _blurTab immediately, so the user clearly sees that the tab will be detached and what the UI will look like upon release. However, in the case of having only one visible tab, if there could be other tab groups, we would immediately switch to tab group view, but then it becomes more difficult for the user to undo that action by dragging the tab back to the tab strip, because it's no longer visible! In addition, tab group view currently doesn't accept TAB_DROP_TYPE drops, and it's strange anyway to undo an action by dragging the tab to another location. Making tab groups global would mitigate this problem, since then we wouldn't have to switch to tab group view. > >+ <method name="_clearTransforms"> > > Needs a more distinctive name, _dragClearTransforms if nothing else; doesn't > need to be super-nice since it's private. Will rename it to _dragClearTransforms. Thanks for the feedback. :)
Comment 180•13 years ago
|
||
> > >+ <method name="_clearTransforms">
> >
> > Needs a more distinctive name, _dragClearTransforms if nothing else; doesn't
> > need to be super-nice since it's private.
>
> Will rename it to _dragClearTransforms.
If you swap the two verb-sounding words, you end up with the much less confusing _clearDragTransforms.
Comment 181•13 years ago
|
||
Addressed Dão's comments. Fixed another bug 465086 regression. In the process of that, fixed bug 652735. (It's more complex not to fix both simultaneously.) (In reply to comment #180) > If you swap the two verb-sounding words, you end up with the much less > confusing _clearDragTransforms. Fixed. Thanks!
Attachment #542302 -
Attachment is obsolete: true
Attachment #543287 -
Flags: review?(dolske)
Attachment #542302 -
Flags: review?(dolske)
Comment 182•13 years ago
|
||
Removed accidental keystrokes on random line of file.
Attachment #543287 -
Attachment is obsolete: true
Attachment #543288 -
Flags: review?(dolske)
Attachment #543287 -
Flags: review?(dolske)
Comment 183•13 years ago
|
||
Actually removed accidental keystrokes on random line of file. My text editor and I are not getting along today. Sorry. :(
Attachment #543288 -
Attachment is obsolete: true
Attachment #543289 -
Flags: review?(dolske)
Attachment #543288 -
Flags: review?(dolske)
Comment 184•13 years ago
|
||
The drag & drop API did not provide the performance we needed, and it caused glitches like bug 666864, so I switched to using plain mouse events, which enabled an implementation that is noticeably more responsive, especially in the common case of rearranging tabs. Using this approach removes the dependencies on bug 533460 and bug 666864. IMHO, it's a cleaner approach, because it only makes sense for Firefox tab strips to accept the drops of Firefox tabs, so no other elements should react to a Firefox tab being dragged over it. For example, using the drag & drop API, the OS X dock popped up when dragging to the bottom of the screen, even though it couldn't accept the drop of a tab. The new patch doesn't have this problem. It turns out that attaching a mousemove event listener to the document in a dragstart listener also captures events outside of the window, which is what we want. There is no need for element.setCapture() because we actually want the original targets for each event. I used dragstart instead of mousedown to initialize the drag, because it provides a convenient cursor jitter threshold before it is triggered.
Attachment #543289 -
Attachment is obsolete: true
Attachment #543394 -
Flags: review?(dolske)
Attachment #543289 -
Flags: review?(dolske)
Updated•13 years ago
|
Comment 185•13 years ago
|
||
(In reply to comment #179) > (In reply to comment #178) > > > >+.tabbrowser-tab[selected] { > > >+ z-index: 2; > > >+} > > > > It's not clear what this is supposed to do, needs a comment. > > Will add the comment: /* keep selected tab on top during drags */ Why is this applied regardless of the tab being dragged?
Comment 186•13 years ago
|
||
@Frank , when will it land in ux branch? , need to try it out , sounds awesome by the way :)
Comment 187•13 years ago
|
||
(In reply to comment #185) > (In reply to comment #179) > > (In reply to comment #178) > > > > > >+.tabbrowser-tab[selected] { > > > >+ z-index: 2; > > > >+} > > > > > > It's not clear what this is supposed to do, needs a comment. > > > > Will add the comment: /* keep selected tab on top during drags */ > > Why is this applied regardless of the tab being dragged? Because it doesn't really make a difference either way when the tab is not being dragged. Do you have a good reason for making the selector more complex and limiting it to |.tabbrowser-tabs[drag] > .tabbrowser-tab[selected]| ? (In reply to comment #186) > @Frank , when will it land in ux branch? , need to try it out , sounds > awesome by the way :) Probably tomorrow. Instead of writing another comment unrelated to development that causes an email to be sent out to 155 people, please check the logs yourself: https://hg.mozilla.org/projects/ux/log?rev=frank
Comment 188•13 years ago
|
||
(In reply to comment #187) > (In reply to comment #185) > > (In reply to comment #179) > > > (In reply to comment #178) > > > > > > > >+.tabbrowser-tab[selected] { > > > > >+ z-index: 2; > > > > >+} > > > > > > > > It's not clear what this is supposed to do, needs a comment. > > > > > > Will add the comment: /* keep selected tab on top during drags */ > > > > Why is this applied regardless of the tab being dragged? > > Because it doesn't really make a difference either way when the tab is not > being dragged. Do you have a good reason for making the selector more > complex and limiting it to |.tabbrowser-tabs[drag] > > .tabbrowser-tab[selected]| ? It's just confusing. I don't see any complexity in the full selector.
Comment 189•13 years ago
|
||
Addressed Dão's comment. Fixed an if condition that caused tab sizing to unlock when it shouldn't have. Removed an unnecessary if condition.
Attachment #543394 -
Attachment is obsolete: true
Attachment #543421 -
Flags: review?(dolske)
Attachment #543394 -
Flags: review?(dolske)
Comment 190•13 years ago
|
||
Comment on attachment 543421 [details] [diff] [review] patch v8 [ui-r=limi] Review of attachment 543421 [details] [diff] [review]: ----------------------------------------------------------------- I didn't do a nitty-gritty review of the math stuff yet, just from a general view it's looking ok. Alas, r- for the issues noted in tabbrowser.xml. ::: browser/base/content/tabbrowser.xml @@ +1485,5 @@ > if (!this._beginRemoveTab(aTab, false, null, true)) > return; > > + if (aTab.selected && this.tabContainer.hasAttribute("drag")) > + this._endTabDrag(aTab); Should be this.tabContainer._endTabDrag? [This was the bug we found... Press Command-W while dragging a tab, drop it at the end, weird stuff happens.] @@ +2782,5 @@ > <field name="_tabDropIndicator"> > document.getAnonymousElementByAttribute(this, "anonid", "tab-drop-indicator"); > </field> > > + <method name="_positionDropIndicator"> Sigh, XBL. It would be nice to keep this code unmoved (for reviewer sanity and blame history), but <handler> vs <method>. :( @@ +2787,5 @@ > + <parameter name="event"/> > + <parameter name="scrollOnly"/> > + <body><![CDATA[ > + if (event.dataTransfer) > + var effects = this._setEffectAllowedForDataTransfer(event); Init effects to "" or in an else here? @@ +2920,5 @@ > + event.screenX >= bo.screenX + bo.width || > + event.screenY < bo.screenY - (detached ? 0 : 1) * bo.height || > + event.screenY >= bo.screenY + (detached ? 1 : 2) * bo.height) { > + state = true; > + if (!this._isEventInsideWindow(event, window)) { From Friday review: |window| is an unused arg here. Consider caching result in a local var earlier, to avoid calling twice? @@ +2924,5 @@ > + if (!this._isEventInsideWindow(event, window)) { > + // cycle through windows from front to back: > + // if the cursor is inside the tab strip of a window, > + // indicate where the tab would be dropped > + let windows = Services.wm.getZOrderDOMWindowEnumerator("navigator:browser", true); So... Bug 462222 talks about this being broken on Mac and Linux (and OS/2). :( It seems those implementations just use an oldest-to-newest window enumerator to fake it, instead of a real top-to-bottom mechanism. This sorta worked in the past because it was typically just used for get the topmost-ish window to open a tab in, but it looks like this could be pretty bad for your usecase. nsIWindowMediator has some other z-order checking methods, but they're all marked [noscript] (and I'm not sure if they're also broken!). You may need to do some widget hacking to figure this out. I'd sorta expect it shouldn't be that hard, but just hasn't been important until now. Also, shouldn't something be checking to see if there's some other window obscuring the background Firefox tabstrip? @@ +3717,5 @@ > + > + document.addEventListener("mousemove", this); > + document.addEventListener("mouseup", this); > + this.mTabstrip.addEventListener("scroll", this); > + tab.addEventListener("TabAttrModified", this); It would be good to add a comment (and maybe test!) explaining when this case can happen. @@ +3736,5 @@ > + // valid urls don't contain spaces ' '; if we have a space it isn't a valid url. > + // Also disallow dropping javascript: or data: urls--bail out > + if (!url || !url.length || url.indexOf(" ", 0) != -1 || > + /^\s*(javascript|data):/.test(url)) > + return; Hmm, this should probably be a whitelist to only allow certain URLs. New bug, natch.
Attachment #543421 -
Flags: review?(dolske) → review-
Comment 191•13 years ago
|
||
I'm getting two issues : 1.The detached tab is leaving behind a 4px tab on the tab strip (always). 2.The tab panel of the detached tab is randomly sometimes white/blank
Comment 192•13 years ago
|
||
(In reply to comment #190) > > + <method name="_positionDropIndicator"> > > Sigh, XBL. It would be nice to keep this code unmoved (for reviewer sanity > and blame history), but <handler> vs <method>. :( Indeed. I call it in two places, so I had to move it. > > + let windows = Services.wm.getZOrderDOMWindowEnumerator("navigator:browser", true); > > So... Bug 462222 talks about this being broken on Mac and Linux (and OS/2). > [...] > Also, shouldn't something be checking to see if there's some other window > obscuring the background Firefox tabstrip? Yeah, I'll have to look into these issues. :( > > @@ +3736,5 @@ > > // valid urls don't contain spaces ' '; if we have a space it isn't a valid url. > > // Also disallow dropping javascript: or data: urls--bail out > > if (!url || !url.length || url.indexOf(" ", 0) != -1 || > > /^\s*(javascript|data):/.test(url)) > > return; > > Hmm, this should probably be a whitelist to only allow certain URLs. New > bug, natch. That was just a de-indentation of old code. I filed bug 669046 to fix it. The other review comments have been addressed. I'm not yet requesting review, because the z-order issue still needs to be resolved. I have ideas for it though.
Attachment #543421 -
Attachment is obsolete: true
Comment 193•13 years ago
|
||
(In reply to comment #191) > I'm getting two issues : > 1.The detached tab is leaving behind a 4px tab on the tab strip (always). > 2.The tab panel of the detached tab is randomly sometimes white/blank 3. The taskbar Preview Has stopped showing tab thumbnail images (only loading icon shown)and this is happening since the launch of tab animation. I've kept cache frequency to 0 for realtime.
Comment 194•13 years ago
|
||
Thank you so much for deliverance from the lags when dragging tabs.
Comment 195•13 years ago
|
||
There is no way to drag the inactive tab
Comment 196•13 years ago
|
||
Sorry to derail the conversation, but people seem to be able to test this bugfix, and I cannot for the life of me figure out how. I'm new to Bugzilla, and I want to be able to test the bugs. Thanks.
Comment 198•13 years ago
|
||
(In reply to comment #196) > Sorry to derail the conversation, but people seem to be able to test this > bugfix, and I cannot for the life of me figure out how. I'm new to Bugzilla, > and I want to be able to test the bugs. Thanks. Latest UX Branch is available here (yesterday Tab Animation was finally smooth): https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-ux/
Comment 199•13 years ago
|
||
(In reply to comment #198) > Latest UX Branch is available here (yesterday Tab Animation was finally > smooth): > https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-ux/ Well, I tried it. Apart from the (already mentioned) inability to drag to bookmarks, I can't alt+tab between firefox windows, which I think is an even more annoying regression. I try to drag to bookmarks, which creates a new windows instead, but I can't even move it back to the original window anymore, without resizing and arranging the widows properly. Also in this case the old drop arrow appears instead of the new animation...
Comment 200•13 years ago
|
||
(In reply to comment #199) > bookmarks, I can't alt+tab between firefox windows, which I think is an even > more annoying regression. I try to drag to bookmarks, which creates a new > windows instead, but I can't even move it back to the original window > anymore, without resizing and arranging the widows properly. Also in this > case the old drop arrow appears instead of the new animation... This sounds pretty bad. Alt+Tab is indeed an important tool when dragging between windows, especially maximized windows.
Comment 201•13 years ago
|
||
Are you going to make an option not to create new windows via detaching at all? To my mind to drag to bookmarks is MUCH MORE important thing than to create a new window (by the way, I don't use it at all). Thus I would like to turn this off and simply to have nice animation when I reorder tabs.
Comment 202•13 years ago
|
||
(In reply to comment #201) > drag to bookmarks is MUCH MORE important FWIW, you can also bookmark a tab by dragging its identity box to bookmarks toolbar.
Comment 203•13 years ago
|
||
I know there are several ways to add a bookmark e.g. using a button on the address bar or, as it's described a few lines above, dragging its identity box or maybe something else but the way I used (dragging a tab) is the most convenient for me. Also when I browse quickly the mouse drag movement can drop and I may by mistake create really annoying new windows. Oh, I know that some people may like this feature (I don't ask you to turn this on by default), but I don't. I like Firefox (maybe like many users) because it have a LOT of options allowing to configure its behavior which include tab behavior too, and I think such option is really necessary and not difficult to do. Excuse me if you don't think so.
Comment 204•13 years ago
|
||
I think it's out of the scope of this bug since this feature is already present in Firefox 4.
Comment 205•13 years ago
|
||
(In reply to comment #203) > I know there are several ways to add a bookmark e.g. using a button on the > address bar or, as it's described a few lines above, dragging its identity > box or maybe something else but the way I used (dragging a tab) is the most > convenient for me. Also when I browse quickly the mouse drag movement can > drop and I may by mistake create really annoying new windows. Oh, I know > that some people may like this feature (I don't ask you to turn this on by > default), but I don't. I like Firefox (maybe like many users) because it > have a LOT of options allowing to configure its behavior which include tab > behavior too, and I think such option is really necessary and not difficult > to do. Excuse me if you don't think so. All this is irrelevant to this bug , please do not discuss about bookmarking here. Also , accidentally making new windows is no bug , its just like accidentally closing firefox etc
Comment 206•13 years ago
|
||
I'm very much enjoying to look & feel of the new animations! However, I've found a problem regarding window sizes on Windows Vista: 1. In a maximized window, open a new tab. 2. Drag the tab down into the content window and release to open it into a new browser window. 3. The new window is maximized; restore it. 4. Drag the tab back into the maximized window. 5. Restore the maximized window. You will now see that the window has been resized to the screen resolution. It makes more sense to me that the window's restore size is left unchanged.
Comment 207•13 years ago
|
||
- Addressed review comments. - Addressed obscured window problem by fixing bug 597989. - Removed bogus tab stub on Windows. - Restored the ability to cancel the drag by pressing the esc key, even while the drag image is not visible or another window is focused. - Mitigated problems with the z-order window enumerator.
Attachment #543660 -
Attachment is obsolete: true
Attachment #544950 -
Flags: review?(dolske)
Comment 208•13 years ago
|
||
Comment on attachment 544950 [details] [diff] [review] patch v10 [ui-r=limi] Oops: > +.tabbrowser-tabs[drag=detach] > .tabbrowser-tab[selected], > +.tabbrowser-tabs[drag=teleport] > .tabbrowser-tab[selected] { The above two lines should be replaced by the following: > +.tabbrowser-tabs[drag=detach] > .tabbrowser-tab[selected] { ("teleport" was a placeholder value used in an approach that I abandoned.)
Comment 209•13 years ago
|
||
Is Alt+Tab for moving tabs between windows fixed?
Comment 210•13 years ago
|
||
(In reply to comment #209) > Is Alt+Tab for moving tabs between windows fixed? Now, yes, at the cost of being able to cancel the drag with the esc key, because setting noautohide to true also causes that shortcut to be disabled.
Attachment #544950 -
Attachment is obsolete: true
Attachment #544974 -
Flags: review?(dolske)
Attachment #544950 -
Flags: review?(dolske)
Comment 211•13 years ago
|
||
Can we finally merge this into trunk?
Comment 212•13 years ago
|
||
It turns out that <panel level="top"/> doesn't work on Linux, so I had to put in an #ifdef'd workaround for that. The primary change from v10 to v11 is the vast improvement in theming. The drag image now approximates the shape of a window or tab correponding to the current drop target. So far, it looks best on Windows Vista/7 for which borderless, transparent panels are easiest to create. Further cosmetic improvements will be left to a followup. I'll upload a screencast of this patch in action on Windows 7 shortly.
Attachment #544974 -
Attachment is obsolete: true
Attachment #545128 -
Flags: review?(dolske)
Attachment #544974 -
Flags: review?(dolske)
Comment 213•13 years ago
|
||
If it lands in a few days on m-c , will it make it through for firefox 7 ?
Comment 214•13 years ago
|
||
Comment 215•13 years ago
|
||
(In reply to comment #213) > If it lands in a few days on m-c , will it make it through for firefox 7 ? No, Firefox 7 moved to Aurora on 2011-07-05, mozilla-central work is now for Firefox 8. See: https://wiki.mozilla.org/Releases https://wiki.mozilla.org/RapidRelease/Calendar
Comment 216•13 years ago
|
||
in the screencast of v12 when we detach the tab from firefox windows the cursor comes to the center of the miniature...it's not better to cursor stay in the top of miniature?
Comment 217•13 years ago
|
||
Attachment #545128 -
Attachment is obsolete: true
Attachment #545561 -
Flags: review?(dolske)
Attachment #545128 -
Flags: review?(dolske)
Comment 218•13 years ago
|
||
(In reply to comment #214) > Created attachment 545132 [details] > Screencast of patch v12 on Windows 7 Very nice! One note though, when we go from window to tab (0:11 in the video), is it possible to make it a "real" tab, that moves other tabs away. It looks like a flying label now.
Comment 219•13 years ago
|
||
The real Tab would be very nice , instead of flying label. Two issues I am having on Windows 7 (with the latest ux nightly build) 1. The new tab button disappears as soon as I detach a tab . 2. Not able to drag the detached tab panel to the task bar. This scenario occurs when the detached tab is from a maximized window and I want it to drag and drop it to another maximized window. Thus I have to hover the mouse to the corresponding taskbar button, but I'm unable to do that as the detached tab is not entering the taskbar.
Comment 220•13 years ago
|
||
Comment 221•13 years ago
|
||
(In reply to comment #220) > Created attachment 545668 [details] > The thumbnail while detaching goes transparent on light backgrounds It would be appreciated if you could convert videos to WebM in future. :) Miro make a good converter that you can use. http://www.mirovideoconverter.com/
Comment 222•13 years ago
|
||
apologizes , i didnt have any nice converter atm , i was thinking to convert it to GIF
Comment 223•13 years ago
|
||
(In reply to comment #152) > I found something : You could call it a bug or a missing feature. > > When we have 2 windows of firefox open. If we detach one tab from one of the > window(say window 1) and drag it to the tab strip of the other window(say > windows 2), the tab from the tab strip of window 1 is removed , but no tab > is added to the tab strip of window 2. Just the arrow is shown. > > What is required would be something like this : > we detach a tab from window 1 , the tab from the tab strip of Window 1 > immediately disappears. And when we add that tab to Window 2, its tab should > be added to Window 2's tab strip along with the tab rearrangement animation > in action. > > I hope my idea is clear to all and I wish to see this feature in this > animation. When will this feature be implemented? It think it's cool and Chrome already has it. I wish to see this feature too.
Comment 224•13 years ago
|
||
It would be great if the window disappears when the last tab is detached like in Chrome. Otherwise very good work, the animations are smooth :)
Comment 225•13 years ago
|
||
Can anyone reproduce the bug that the new tab button disappears on tab detach ? Also that the detached panel cannot be moved on to taskbar in windows 7
Comment 226•13 years ago
|
||
(In reply to comment #225) > Can anyone reproduce the bug that the new tab button disappears on tab > detach ? > Also that the detached panel cannot be moved on to taskbar in windows 7 Yes, I have the same bug in Windows 7. I think everyone can reproduce this.
Comment 227•13 years ago
|
||
(In reply to comment #226) > (In reply to comment #225) > > Can anyone reproduce the bug that the new tab button disappears on tab > > detach ? > > Also that the detached panel cannot be moved on to taskbar in windows 7 > > Yes, I have the same bug in Windows 7. I think everyone can reproduce this. I can reproduce it too.
Comment 228•13 years ago
|
||
I can reproduce it on the Mac build as well. The New Tab button disappears on tab detach and I can't drag a tab over the Dock. Hovering the Dock while dragging a tab is as important as the Windows 7 Taskbar, since it's one of the ways to trigger Application-specific Exposé and have it showcase all the application's windows miniatures on desktop, to pick the correct one to attach the tab to.
Comment 229•13 years ago
|
||
(In reply to comment #218) > One note though, when we go from window to tab (0:11 in the video), is it > possible to make it a "real" tab, that moves other tabs away. It looks like > a flying label now. We already have plans for many more improvements including this after landing this first. > 1. The new tab button disappears as soon as I detach a tab . Intentional behavior to mitigate issue noted in comment 136 until followup work is completed. > 2. Not able to drag the detached tab panel to the task bar. This scenario > occurs when the detached tab is from a maximized window and I want it to > drag and drop it to another maximized window. Thus I have to hover the mouse > to the corresponding taskbar button, but I'm unable to do that as the > detached tab is not entering the taskbar. Alt+tab. (Cmd+` on OS X.) (In reply to comment #220) > Created attachment 545668 [details] > The thumbnail while detaching goes transparent on light backgrounds Thanks for the catch. I'll try to fix this. Please convert these to WebM using the awesome, free Miro Video Converter.
Comment 230•13 years ago
|
||
(In reply to comment #229) > Alt+tab. (Cmd+` on OS X.) Yes, as a temporary workaround it's okay (or use the keyboard shortcut set for Exposé, which is what I'm doing right now), but it'd be nice as a future improvement if one could use the Dock/Taskbar without having to reach for the keyboard.
Comment 231•13 years ago
|
||
On my Vista installation, dragging the tab preview tab initially works excellently, until pressing Alt+Tab to switch to a different Fx window. Occasionally, the dragged window then remains stationary until I move the cursor outside of the window. I find this very hard to reproduce, but it seems that when it happens, both windows are maximized. I've also noticed that if I move my mouse very abruptly, my cursor will exit the window and the dragged tab will not follow it until I mouse-over it again.
Comment 232•13 years ago
|
||
> 2. Not able to drag the detached tab panel to the task bar. This scenario
> occurs when the detached tab is from a maximized window and I want it to
> drag and drop it to another maximized window. Thus I have to hover the mouse
> to the corresponding taskbar button, but I'm unable to do that as the
> detached tab is not entering the taskbar.
It's also connected with inability to Show Desktop, even if mouse is pointing at the "Show Desktop" button.
Comment 233•13 years ago
|
||
(In reply to comment #220) > Created attachment 545668 [details] > The thumbnail while detaching goes transparent on light backgrounds I can't reproduce this. Are you running any theme or appearance-related extensions? Could you temporarily removing everything (including any @namespace lines) from your profile's userChrome.css file and simply putting the following line in the file: .tab-drag-thumbnail { background-color: white; background-clip: content-box; }
Comment 234•13 years ago
|
||
(In reply to comment #233) > (In reply to comment #220) > > Created attachment 545668 [details] > > The thumbnail while detaching goes transparent on light backgrounds > > I can't reproduce this. Are you running any theme or appearance-related > extensions? As a Windows developer I recognize this phenomenon: it is caused by Aero Glass, which uses black as glassy transparency. There's a resolution to this, but I only have Java native interface code, which I imagine isn't very useful. You could best reproduce it by dragging a tab with a mostly black preview.
Comment 235•13 years ago
|
||
(In reply to comment #234) > As a Windows developer I recognize this phenomenon: it is caused by Aero > Glass, which uses black as glassy transparency. [...] > > You could best reproduce it by dragging a tab with a mostly black preview. I'm aware of the phenomenon. We ran into that problem a lot in our early attempts to make Firefox 4 use Aero Glass. I was unable to reproduce it by dragging tabs with thumbnails of any color. If you can reproduce this bug, could you navigate to about:support and paste the text in the Graphics section in a comment here?
Comment 236•13 years ago
|
||
(In reply to comment #235) > If you can reproduce this bug, could you navigate to about:support and paste > the text in the Graphics section in a comment here? Graphics Adapter Description ATI Radeon HD 4800 Series Vendor ID 1002 Device ID 9442 Adapter RAM 512 Adapter Drivers aticfx64 aticfx64 aticfx64 aticfx32 aticfx32 aticfx32 atiumd64 atidxx64 atidxx64 atiumdag atidxx32 atidxx32 atiumdva atiumd6a atitmm64 Driver Version 8.861.0.0 Driver Date 5-24-2011 Direct2D Enabled true DirectWrite Enabled true (6.1.7600.16699) ClearType Parameters Gamma: 2200 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 300 WebGL Renderer Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.686) GPU Accelerated Windows 1/1 Direct3D 10
Comment 237•13 years ago
|
||
I can too Graphics: Adapter Description ATI Mobility Radeon HD 5650 Vendor ID 1002 Device ID 68c1 Adapter RAM 1024 Adapter Drivers atiu9p64.dll atiuxp64 atiuxp64 atiu9pag atiuxpag atiuxpag atiumdva atiumd6a atitmm64 Driver Version 8.771.1.0 Driver Date 9-9-2010 Direct2D Enabled true DirectWrite Enabled true (6.1.7601.17563) ClearType Parameters ClearType parameters not found WebGL Renderer Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.686) GPU Accelerated Windows 1/1 Direct3D 10
Comment 238•13 years ago
|
||
Graphics: Adapter Description NVIDIA GeForce 8600 GTS Vendor ID 10de Device ID 0400 Adapter RAM 256 Adapter Drivers nvd3dum nvwgf2um,nvwgf2um Driver Version 8.17.12.6724 Driver Date 2-23-2011 Direct2D Enabled true DirectWrite Enabled true (7.0.6002.18409) ClearType Parameters ClearType parameters not found WebGL Renderer Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.686) GPU Accelerated Windows 1/1 Direct3D 10
Comment 239•13 years ago
|
||
I can reproduce the thumbnail issue. Also the animations are a little bit sluggish. Adapter Description: NVIDIA GeForce 8800 GT Vendor ID: 10de Device ID: 0611 Adapter RAM: 512 Adapter Drivers: nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um Driver Version: 8.17.12.7533 Driver Date: 5-20-2011 Direct2D Enabled: true DirectWrite Enabled: true (6.1.7601.17563) ClearType Parameters: ClearType parameters not found WebGL Renderer: Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.686) GPU Accelerated Windows: 1/1 Direct3D 10
Comment 240•13 years ago
|
||
I can't reproduce this bug. Looks like the difference here is that Direct2D and DirectWrite are disabled on my system. I don't see how DirectWrite can affect this, so perhaps the problem lies in Direct2D? Graphics Adapter Description ATI Mobility Radeon X1300 Vendor ID 1002 Device ID 714a Adapter RAM Unknown Adapter Drivers atiumdag atiumdva atitmmxx Driver Version 8.561.0.0 Driver Date 12-1-2008 Direct2D Enabled Blocked for your graphics driver version. Try updating your graphics driver to version 10.6 or newer. DirectWrite Enabled false (6.1.7600.16763) ClearType Parameters ClearType parameters not found WebGL Renderer Blocked for your graphics driver version. Try updating your graphics driver to version 10.6 or newer. GPU Accelerated Windows 0/1. Blocked for your graphics driver version. Try updating your graphics driver to version 10.6 or newer.
Comment 241•13 years ago
|
||
(In reply to comment #195) > There is no way to drag the inactive tab This still seems to be an issue (on Mac at least). Specifically: - If the current tab is the leftmost one and you drag any other tab to move it, nothing will happen - If the current tab isn't the leftmost one and you drag any other tab to move it, the current tab will jump to the leftmost position http://alimonda.com/firefoxtabdrag.webm
Comment 242•13 years ago
|
||
(In reply to comment #241) > (In reply to comment #195) > > There is no way to drag the inactive tab > > This still seems to be an issue (on Mac at least). > > Specifically: > - If the current tab is the leftmost one and you drag any other tab to move > it, nothing will happen > - If the current tab isn't the leftmost one and you drag any other tab to > move it, the current tab will jump to the leftmost position > > http://alimonda.com/firefoxtabdrag.webm In windows too
Comment 243•13 years ago
|
||
(In reply to comment #242) > In windows too After checking other people's experiences on the mozillazine UX branch thread ([1]), it ended up being an extension's fault. It's likely to be the same for you, if you use Tab Utilities or a similar addon. WFM on a clean profile. [1]: http://forums.mozillazine.org/viewtopic.php?p=11027729#p11027729
Comment 244•13 years ago
|
||
Fixed the transparent panel glitch caused by bizarre Aero Glass interactions. Thanks for listening your graphics driver information. We don't need any more. The drag preview now uses a live thumbnail! The drag preview on OS X looks more like a window. The new tab button is hidden only when there are no remaining visible tabs. Tabs animate to fill the space of a tab being detached.
Attachment #463754 -
Attachment is obsolete: true
Attachment #477449 -
Attachment is obsolete: true
Attachment #545132 -
Attachment is obsolete: true
Attachment #545561 -
Attachment is obsolete: true
Attachment #545668 -
Attachment is obsolete: true
Attachment #546359 -
Flags: review?(dolske)
Attachment #545561 -
Flags: review?(dolske)
Comment 245•13 years ago
|
||
Comment on attachment 546359 [details] [diff] [review] patch v14 [ui-r=limi] >--- a/browser/themes/winstripe/browser/browser-aero.css >+++ b/browser/themes/winstripe/browser/browser-aero.css >+%define panelBorder rgba(10%,10%,10%,.7) rgba(255,255,255,.7) >+%define panelGlass rgba(255,255,255,.3) rgba(255,255,255,.3) rgba(255,255,255,.3) rgba(255,255,255,.3) >+%define previewBorder rgba(20%,20%,20%,.7) >+ .tab-drag-panel { >+ -moz-appearance: -moz-win-borderless-glass; >+ border-radius: 7px; >+ } >+ .tab-drag-label { >+ padding: 4px; >+ background-color: -moz-dialog; >+ border: 1px solid DimGray; >+ border-radius: 3px; >+ } >+ .tab-drag-panel:not([target]) > .tab-drag-label { >+ display: none; >+ } >+ .tab-drag-preview { >+ display: block; >+ margin: 5px; >+ box-shadow: 0 0 5px rgba(10%,10%,10%,.7); >+ border-width: 15px 7px 7px; >+ border-style: solid; >+ -moz-border-top-colors: @panelBorder@ @panelGlass@ @panelGlass@ @panelGlass@ @previewBorder@; >+ -moz-border-right-colors: @panelBorder@ @panelGlass@ @previewBorder@; >+ -moz-border-bottom-colors: @panelBorder@ @panelGlass@ @previewBorder@; >+ -moz-border-left-colors: @panelBorder@ @panelGlass@ @previewBorder@; >+ border-radius: 7px; >+ } >+ >+ .tab-drag-preview::before { >+ content: ""; >+ display: block; >+ margin: -14px 0 0 -2px; >+ width: 32px; >+ height: 7px; >+ background-image: -moz-linear-gradient(rgb(247,182,82), rgb(215,98,10) 95%); >+ background-clip: content-box; >+ border: 2px solid; >+ border-top-width: 0; >+ -moz-border-right-colors: rgba(255,255,255,.3) rgba(0,0,0,.7); >+ -moz-border-bottom-colors: rgba(255,255,255,.3) rgba(0,0,0,.7); >+ -moz-border-left-colors: rgba(255,255,255,.3) rgba(0,0,0,.7); >+ border-radius: 0 0 3px 3px; >+ } My God! What exactly is this stuff doing?
Comment 246•13 years ago
|
||
(In reply to comment #245) > >--- a/browser/themes/winstripe/browser/browser-aero.css > >+++ b/browser/themes/winstripe/browser/browser-aero.css > >[...] > > My God! What exactly is this stuff doing? It draws a window frame around the drag thumbnail. The ::before style draws a miniature orange appmenu button. Will add much needed comments before checking in.
Comment 247•13 years ago
|
||
(In reply to comment #246) > (In reply to comment #245) > > >--- a/browser/themes/winstripe/browser/browser-aero.css > > >+++ b/browser/themes/winstripe/browser/browser-aero.css > > >[...] > > > > My God! What exactly is this stuff doing? > > It draws a window frame around the drag thumbnail. Does -moz-appearance: -moz-win-borderless-glass; not draw a window frame? > The ::before style draws a miniature orange appmenu button. Quite a hack, questionable benefit :/
Comment 248•13 years ago
|
||
Accounted for OS X panel repositioning. Fixed a selector on Windows. Added comments to Windows theming code. (In reply to comment #247) > Does -moz-appearance: -moz-win-borderless-glass; not draw a window frame? Yes. However, it does not allow the elements to be drawn inside the frame to create a more stylized panel such as the one shown here: http://www.stephenhorlander.com/images/blog-posts/tab-animation/animation-tab-tear-off.html We don't want Firefox to appear generic. > > The ::before style draws a miniature orange appmenu button. > > Quite a hack, questionable benefit :/ The intentionally recognizable button makes it clear that the panel represents and will "transform" into a new window upon drop. If we decide on an absolute size for the panel that is independent of window and screen size, then we can easily use -moz-border-image to draw an arbitrary non-overlapping frame.
Attachment #546359 -
Attachment is obsolete: true
Attachment #546389 -
Flags: review?(dolske)
Attachment #546359 -
Flags: review?(dolske)
Comment 249•13 years ago
|
||
(In reply to comment #248) > > Does -moz-appearance: -moz-win-borderless-glass; not draw a window frame? > > Yes. However, it does not allow [...] Why is it in the stylesheet then?
Comment 250•13 years ago
|
||
When dragging a tab out of the window, the tab shown in the browser should change, like in Chrome and IE. When you drag a tab out of a window where there is only one tab, the window should be hidden (like in Chrome and IE). The live tab also causes FishIE to go from 60 FPS to something like 10. Not sure whether this is a bug. Looking good though. Like the window frame, though you may be able to slip "Firefox" inside the orange box.
Comment 251•13 years ago
|
||
If you are trying to make the detached tab look like a window, then it would be nice to have it look like a tab , just like chrome makes a detached tab look like a tab. Also it would be nice if the detached tab could be dragged over the taskbar (in windows) and dock (in mac) . Atl+ TAB is a temporary solution, also there is no proper indication that the usre can Alt+Tab to go to the next maximized firefox window.
Comment 252•13 years ago
|
||
Mozilla - I commend you for all the hard work you are putting into this project and for resolving bug issues. I am rooting for completion within Firefox 8. I have noticed a minor issue with the tab previews when the sites involve Flash objects and am using the 7/17/11 User Interface Nightly. If I have www.gsn.com open and drag the tab down to make another window I can see the Flash object (top slideshow with arrows) appear in the mini preview. However, if I have ANY of the following sites open (Crossword puzzles which use Flash): 1) http://games.latimes.com/index_crossword.html?uc_feature_code=tmcal 2) http://www.patsajakgames.com/games/psgcc_online.html 3) http://www.patsajakgames.com/games/luckyletters_online.html 4) http://www.flashgames247.com/play/13922.html I don't see the Flash objects in the mini tab drag preview. Screenshots are provided: "Flash Object Missing" = Site 1 above "Flash Object Appears" = www.gsn.com
Comment 253•13 years ago
|
||
Comment 254•13 years ago
|
||
Comment 255•13 years ago
|
||
Frank, these latest changes are great. I love the little orange Firefox button in the tear-off window. That adds so much context that I no longer lean towards showing the tab as context in the tear-off like the other browsers do. There is really only one (or maybe two) style issue I see left. The glass-like window border on the tear-off window is completely transparent when it should have some opacity or blur or opacity so the content behind it doesn't peek through so strong. The window frame should also picked up the window color from the system (the default on Win7 is "sky" so we could at least fake that if the actual window color isn't available to us via css. Arrow panels currently fake this color, I believe, because it doesn't change when I change window colors.) Kudos again on the Firefox button. I think that makes a huge difference how the tear off feature feels. It really conveys that what you're dragging out is a window and not some foreign widget.
Comment 256•13 years ago
|
||
Frank, sorry about that last comment. I see we're waiting on the color to be available via CSS at bug 578780. Is there anything we can do in the mean time to fake it like the Arrow panel does. That fakes the color but not the opacity/blur. Is there anything we can do to make the glass-like portions semi-opaque?
Comment 257•13 years ago
|
||
OK. I've looked at Arrow panels and this doesn't seem like an super-simple change so let's wait and I'll file follow-up bugs. Let's get what we have here reviewed and landed and deal with the edge cases and style clean-up in new bugs.
Comment 258•13 years ago
|
||
Bug: 1. Set the Firefox window to a certain width 2. Drag a tab out, don't release, put it back in the tab bar 3. Reduce Firefox's width It doesn't reduce beyond the width.
Comment 259•13 years ago
|
||
(In reply to comment #249) > (In reply to comment #248) > > > Does -moz-appearance: -moz-win-borderless-glass; not draw a window frame? > > > > Yes. However, it does not allow [...] > > Why is it in the stylesheet then? Using the default -moz-appearance makes the panel opaque. Do you know of a -moz-appearance value that would enable transparency without unnecessary Aero Glass and border effects? (In reply to comment #252) > I don't see the Flash objects in the mini tab drag preview. This is a "bug" in the API I used for live previews (bug 506826). Whether -moz-element is able to draw the plugin's content is dependent on the plugin content's so-called "windowed" state. Plugins are in charge of rendering their portion of the page, so it's non-trivial to resolve. I'll move the screenshots to a separate bug and ask the platform team what it would take to fix it. (In reply to comment #255) > There is really only one (or maybe two) style issue I see left. The > glass-like window border on the tear-off window is completely transparent > when it should have some opacity or blur or opacity so the content behind it > doesn't peek through so strong. The window frame should also picked up the > window color from the system (the default on Win7 is "sky" so we could at > least fake that if the actual window color isn't available to us via css. > Arrow panels currently fake this color, I believe, because it doesn't change > when I change window colors.) It's not completely transparent. It's white with an opacity of 0.3. For better or worse, we can't blur content that is not ours. The OS does that as part of its window compositing. Perhaps, we could just make it more frosted, since we can't do the blur. > Kudos again on the Firefox button. I think that makes a huge difference how > the tear off feature feels. It really conveys that what you're dragging out > is a window and not some foreign widget. Thanks. A shoutout to Felipe, who helped me out with those latest changes.
Comment 260•13 years ago
|
||
Why can't we use the blur (and border) from -moz-appearance: -moz-win-borderless-glass? This way the mini window could be constructed roughly the same way, the main window is, and we will be having a nice and native glass effect. This does not work for arrow panels because of the non-rectangular shape (especially the arrow at the top) but I don't see why it wouldn't work here.
Comment 261•13 years ago
|
||
> Using the default -moz-appearance makes the panel opaque. Do you know of a
> -moz-appearance value that would enable transparency without unnecessary
> Aero Glass and border effects?
"none"
Comment 262•13 years ago
|
||
Comment on attachment 546389 [details] [diff] [review] patch v15 [ui-r=limi] Review of attachment 546389 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/winstripe/browser/browser-aero.css @@ +321,5 @@ > + } > + .tab-drag-preview::before { /* draw miniature appmenu button */ > + content: ""; > + display: block; > + margin: -14px 0 0 -2px; Is this going to work correctly in RTL mode too?
Comment 263•13 years ago
|
||
Simplified workaround for broken z-order enumerator on Linux. (In reply to comment #247) (Dão) (In reply to comment #260) (Jonathan) I switched to using the native glass effect, but there's a bug with the border and box shadow. I filed bug 672710 for that. (In reply to comment #262) > Is this going to work correctly in RTL mode too? No, but I fixed it in this version. Thanks for catching it.
Attachment #546389 -
Attachment is obsolete: true
Attachment #546424 -
Attachment is obsolete: true
Attachment #546425 -
Attachment is obsolete: true
Attachment #546965 -
Flags: review?(dolske)
Attachment #546389 -
Flags: review?(dolske)
Comment 264•13 years ago
|
||
The thumbnail has an unneeded grey square corner outside the bottom-right corner of the aero border. http://tinyurl.com/3n8znwe Also, the animations are little bit sluggish and the tabs stuck for a little time when moved first.
Comment 265•13 years ago
|
||
I'm sorry, first is Bug 458864...
Comment 266•13 years ago
|
||
Moved generic Windows style rules from browser-aero.css to browser.css. Optimized code that iterates through windows front-to-back. Not yet obsoleting patch v16 in case dolske already started reviewing it.
Attachment #547135 -
Flags: review?(dolske)
Comment 267•13 years ago
|
||
from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7
Comment 268•13 years ago
|
||
(In reply to comment #267) > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7 I watched the video about five times.....what exactly are we looking at / the issue here??
Comment 269•13 years ago
|
||
Yep, better communication please. At first look I saw a blank tab before app tabs.
Comment 270•13 years ago
|
||
(In reply to comment #268) > (In reply to comment #267) > > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7 > > I watched the video about five times.....what exactly are we looking at / > the issue here?? A non-pinned tab preceding pinned tabs. (In reply to comment #269) > Yep, better communication please. Certainly a great example for good communication, that sentence.
Comment 271•13 years ago
|
||
I guess the unspoken question was 'how did that happen?' Anyway, here's the link to the Mozillazine posts referenced earlier. Might not help entirely, but at least there's a clearer picture there; http://forums.mozillazine.org/viewtopic.php?f=23&t=2230295&start=263
Comment 272•13 years ago
|
||
the detach-preview-window (or whatever its called technically) now uses aero blurred transparency , but dragging it is quite slow compared to dragging original firefox window.
Comment 273•13 years ago
|
||
(In reply to comment #267) > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7 (In reply to comment #271) > I guess the unspoken question was 'how did that happen?' After watching it five times, I still don't understand how to reproduce this. The linked Mozillazine thread seems to be discussing tab duplication, which is unrelated. (In reply to comment #272) > the detach-preview-window (or whatever its called technically) now uses aero > blurred transparency , but dragging it is quite slow compared to dragging > original firefox window. What does "original firefox window" mean in this context?
Comment 274•13 years ago
|
||
(In reply to comment #273) > (In reply to comment #267) > > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7 > > (In reply to comment #271) > > I guess the unspoken question was 'how did that happen?' > > After watching it five times, I still don't understand how to reproduce > this. The linked Mozillazine thread seems to be discussing tab duplication, > which is unrelated. quote: - http://cl.ly/8cxW/Image_2011-07-20_at_8.10.38_PM.png - Google+ was not an app tab. This occurred after going into panorama. I can't reproduce it; I'm really not sure how it happened - Yup, saw it multiple times. Happens when you're quickly dragging stuff
Comment 275•13 years ago
|
||
(In reply to comment #274) > > (In reply to comment #267) > > > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7 > > [...] Happens when you're quickly dragging stuff I've never seen it happen. Any idea why it happens? moveTabTo() is supposed to guard against that. Have you tried it with the latest UX build?
Comment 276•13 years ago
|
||
(In reply to comment #275) > (In reply to comment #274) > > > (In reply to comment #267) > > > > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7 > > > > [...] Happens when you're quickly dragging stuff > > I've never seen it happen. > Any idea why it happens? moveTabTo() is supposed to guard against that. I would think so. > Have you tried it with the latest UX build? In case you were asking me specifically: I haven't seen it myself. (Haven't tried to reproduce it either.)
Comment 277•13 years ago
|
||
(In reply to comment #276) > (In reply to comment #275) > > Any idea why it happens? moveTabTo() is supposed to guard against that. > > I would think so. In that case, they are probably using some add-on. If an add-on is writing their own poor implementation of moveTabTo(), that can't be helped. There have already been "bug reports" in this bug that are actually caused by Tab Utilities.
Comment 278•13 years ago
|
||
(In reply to comment #277) > (In reply to comment #276) > > (In reply to comment #275) > > > Any idea why it happens? moveTabTo() is supposed to guard against that. > > > > I would think so. > > In that case, they are probably using some add-on. If an add-on is writing > their own poor implementation of moveTabTo(), that can't be helped. There > have already been "bug reports" in this bug that are actually caused by Tab > Utilities. (Person that posted the image of the Google+ Tab bug here) This was the most recent UX Build when I posted (late yesterday). My addons include: DOM Inspector, Firebug (1.7), Flashblock, Garmin Communicator, Greasemonkey, Growl For Windows/GNTP, Missing e (Tumblr modifier), Prospector Instant Preview, Prospector Speak Words, Stylish (you can see my customizations in the screenshot), Twitter Address Bar Search, and User Agent RG. The only one of this that I think might impact tabs would be Stylish, but since I can't reproduce reliably, I can't test it
Comment 279•13 years ago
|
||
(In reply to comment #278) > (Person that posted the image of the Google+ Tab bug here) Thanks for commenting. > This was the most recent UX Build when I posted (late yesterday). I pushed out a change yesterday, so today's build is slightly different, although, this problem should have never appeared in any version of the bug's patch. > My addons include: > DOM Inspector, Firebug (1.7), Flashblock, Garmin Communicator, Greasemonkey, > Growl For Windows/GNTP, Missing e (Tumblr modifier), Prospector Instant > Preview, Prospector Speak Words, Stylish (you can see my customizations in > the screenshot), Twitter Address Bar Search, and User Agent RG. > > The only one of this that I think might impact tabs would be Stylish, but > since I can't reproduce reliably, I can't test it I doubt Stylish is causing the problem. I don't know about the other add-ons.
Comment 280•13 years ago
|
||
(In reply to comment #279) > I doubt Stylish is causing the problem. I don't know about the other add-ons. I agree, but it seems most likely out of the others, as the rest don't impact tabs at all. I have a few modifications on tabs in stylish. Would making the margin of .tabbrowser-tab -2px impact the tab location detection so that this could happen? Otherwise, I'm not sure it was an addon. I can't reproduce the quickly dragging tabs to force one tab over like the video, I got this to occur when switching from Panorama, so I'm not sure how related the two are
Comment 281•13 years ago
|
||
(In reply to comment #273) > (In reply to comment #267) > > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7 > > (In reply to comment #271) > > I guess the unspoken question was 'how did that happen?' > > After watching it five times, I still don't understand how to reproduce > this. The linked Mozillazine thread seems to be discussing tab duplication, > which is unrelated. > > (In reply to comment #272) > > the detach-preview-window (or whatever its called technically) now uses aero > > blurred transparency , but dragging it is quite slow compared to dragging > > original firefox window. > > What does "original firefox window" mean in this context? i just mean to say that the detach thumbnail window is slow at dragging. (Original firefox window meant the whole browser window)
Comment 282•13 years ago
|
||
Comment on attachment 547135 [details] [diff] [review] patch v17 [ui-r=limi] Review of attachment 547135 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to comment #243) > (In reply to comment #242) > > In windows too > > After checking other people's experiences on the mozillazine UX branch > thread ([1]), it ended up being an extension's fault. It's likely to be the > same for you, if you use Tab Utilities or a similar addon. WFM on a clean > profile. > > [1]: http://forums.mozillazine.org/viewtopic.php?p=11027729#p11027729 ::: browser/base/content/tabbrowser.xml @@ +2983,5 @@ > + > + <method name="_updateTabDetachState"> > + <parameter name="event"/> > + <body><![CDATA[ > + let draggedTab = this.selectedItem; The patch is assuming that people are always dragging the active tab. This is the fault.
Comment 283•13 years ago
|
||
(In reply to comment #282) > The patch is assuming that people are always dragging the active tab. Yes, the dragged tab should always be the active one, since tabs are selected on mousedown. Dragging background tabs and select on mouseup/click are not interactions we plan to support.
Comment 284•13 years ago
|
||
(In reply to comment #283) > (In reply to comment #282) > > The patch is assuming that people are always dragging the active tab. > > Yes, the dragged tab should always be the active one, since tabs are > selected on mousedown. Dragging background tabs and select on mouseup/click > are not interactions we plan to support. We all understand that Firefox won't support all options by itself, but you should not be aiming at creating inabilities for extensions. You'd better get the dragged tab from some pass-in parameter instead of assuming the active tab. Another logical solution is to save the dragged tab in gBrowser.mContextTab.
Comment 285•13 years ago
|
||
Had to unbitrot patch due to the landing of the patch for bug 649480. For the sake of add-on compatibility, removed assumption that dragged tab is active tab. Fortunately, this comes with zero additional maintenance cost.
Attachment #547614 -
Flags: review?(dolske)
Comment 286•13 years ago
|
||
Comment on attachment 547614 [details] [diff] [review] patch v18 [ui-r=limi] >+#main-window[privatebrowsingmode=temporary] #tabbrowser-tabs > hbox > .tab-drag-panel > .tab-drag-preview::before, What is #tabbrowser-tabs > hbox > .tab-drag-panel > doing here?
Comment 287•13 years ago
|
||
I think drawing a windows aero-glass border outside the preview of tab is cool, but its aim is to show the preview, the fav-icon, the title of the holding tab. So don't draw Firefox button; just the border, title and fav-icon is enough for the user to understand that they are creating a new Firefox window with that page.
Comment 288•13 years ago
|
||
It would be nice if you could press [esc] to cancel detaching the tab. I've also noticed that occasionally my cursor will exit the preview pane and I can't get the tab to nest itself into the original window; releasing the mouse button has no effect as the preview window keeps moving with the cursor. This must sound pretty vague, so let me know if I need to make a screencap.
Comment 289•13 years ago
|
||
(In reply to comment #288) > It would be nice if you could press [esc] to cancel detaching the tab. > > I've also noticed that occasionally my cursor will exit the preview pane and > I can't get the tab to nest itself into the original window; releasing the > mouse button has no effect as the preview window keeps moving with the > cursor. This must sound pretty vague, so let me know if I need to make a > screencap. Yup , I can confirm that , its exactly like you say. Sometimes when trying to detach the tab , it does not tear off. (Now the mouse is away from the tab strip as we tryed to detach the tab). Now we bring the mouse back to tab strip and the tab is magically pinned to the mouse pointer like we are still dragging it around (even if there is no mouse down) . Now unless we double click the tab strip , the tab does not let go off the mouse.
Comment 290•13 years ago
|
||
I've noticed an issue (not at all serious, mind you) with two monitors on Windows 7, where dragging the detached tab to the edge of the screen sort of sticks until the mouse crosses over at which point it jumps to the other screen, so the tab preview panel won't appear on both screens. Like I said, not particularly problematic, just looks a little twitchy.
Comment 291•13 years ago
|
||
I like where the UX build is at now, but believe it still needs to be polished a bit before prime time. I am running Windows 7 64-Bit with 4 GB of RAM and an EVGA GeForce GTS450 (PCI-E 2.0) containing 1 GB of GDDR5 memory and notice sluggishness moving tabs around after having only 12 or 13 tabs open.
Updated•13 years ago
|
Attachment #546965 -
Attachment is obsolete: true
Attachment #546965 -
Flags: review?(dolske)
Updated•13 years ago
|
Attachment #547135 -
Attachment is obsolete: true
Attachment #547135 -
Flags: review?(dolske)
Comment 292•13 years ago
|
||
Comment on attachment 547614 [details] [diff] [review] patch v18 [ui-r=limi] Review of attachment 547614 [details] [diff] [review]: ----------------------------------------------------------------- 1) [Note to self...] The two issues I raised in the previous review are sufficiently addressed now... One was the issue of how to detect when an intermediate, non-Mozilla window is above a background Firefox window (such that if you got unlucky, your fling to detach a tab into a new window actually move it to the background window, and you're left wondering what happened). That's made moot by raising the background window, so that if you move a tab that way the window you've dragged to should always end up on top. The other was the issue with the broken getZOrderDOMWindowEnumerator. It's ok on Windows, bug 450576 now indicates it seems to be working on OS X, and although it's still broken on Linux (bug 156333), I'm satisfied that (1) that bug is getting activity again and (2) the workaround in this patch is sufficient to reduce the user impact to just an annoyance. Specifically, the impact is that the workaround may, sometimes, raise a window that although is correctly under your mouse pointer, is actually behind the window you see. But from some experimentation that's still generally ok -- you already have to carefully aim for the other window's tabstrip to move a tab there, and so even if the wrong window pops forward you're still likely to be concentrating on the window you were aiming for, and if it's sticking out you can still drag over to that to bring it forward and successfully complete the task. Obviously not _great_, but acceptable for now. Additionally, even on platforms with a working getZOrderDOMWindowEnumerator() there are pathological cases where we "correctly" raise a window and make it difficult to get at the window you meant to drag to. Long story short -- still some tweaking to do but the gain is far, far, greater than the pain at this point. 2) Tests. Between soaking on the UX branch recently, a year's worth of test builds, and existing test coverage I'm not too concerned. But since there have been some subtle (but carefully engineered) behaviors added in this patch, it would be good for future sanity to avoid unknowingly breaking them. I'd suggest filing a followup bug with a written description of what you think is good to check for. That allows you/us to have a braindump of what you know to look for now, and serves as a guideline for future testing... Ideally as automated tests, maybe even MozMill tests. Anyway, a few small nits in this review... Fix those + test followup and OMG LAND IT! :) ::: browser/base/content/tabbrowser.css @@ +38,5 @@ > background-color: transparent; > } > > +.tab-drag-preview { > + background: -moz-element(#content) left top; I suppose this won't show the right thing when dragging a background tab? ::: browser/base/content/tabbrowser.xml @@ +2831,5 @@ > + event.preventDefault(); > + event.stopPropagation(); > + > + var tabStrip = this.mTabstrip; > + var ltr = (window.getComputedStyle(this, null).direction == "ltr"); Would |ltr = (window.document.dir == "ltr");| work here? This gets called fairly frequently, so it should be cheaper. Also, getComputedStyle is going to do a style flush, which could be a bunch of work and cause lag (though for all I know we might be causing that elsewhere too, so eliminating this one might not matter (yeah, it looks like we do)). Hmm, seems like this might be interesting fodder for some benchmarking / metrics. :) @@ +2944,5 @@ > + let numPinned = this.tabbrowser._numPinnedTabs; > + let leftmostTab = draggedTab.pinned ? tabs[0] : tabs[numPinned]; > + let rightmostTab = draggedTab.pinned ? tabs[numPinned-1] : tabs[tabs.length-1]; > + let tabWidth = draggedTab.getBoundingClientRect().width; > + let ltr = (window.getComputedStyle(this).direction == "ltr"); Ditto, as above. @@ +2994,5 @@ > + draggedTab._targetWindow = window; > + window.focus(); > + } > + else if (draggedTab._dropTarget) > + draggedTab._dropTarget._tabDropIndicator.collapsed = true; Please brace else-block since the former if-block was braced. @@ +2995,5 @@ > + window.focus(); > + } > + else if (draggedTab._dropTarget) > + draggedTab._dropTarget._tabDropIndicator.collapsed = true; > + delete draggedTab._dropTarget; Is this supposed to be part of the else-block? Seems odd to be nuking it every call... @@ +2997,5 @@ > + else if (draggedTab._dropTarget) > + draggedTab._dropTarget._tabDropIndicator.collapsed = true; > + delete draggedTab._dropTarget; > + > + function isEventOutsideWindow(event, win) Please brace body to make it stand out better. Can any of the other similar "is x bounded by y" checks below make use of this too? Especially if you passed in window.boxObject instead of window? @@ +3007,5 @@ > + let winEnum = Services.wm.getZOrderDOMWindowEnumerator("navigator:browser", true); > + let wins = []; > + while (winEnum.hasMoreElements()) > + wins.push(winEnum.getNext()); > + // Work around broken z-order enumerator on Linux. Please note bug number here. @@ +3013,5 @@ > + winEnum = Services.wm.getEnumerator("navigator:browser"); > + while (winEnum.hasMoreElements()) > + wins.unshift(winEnum.getNext()); > + } > + wins.every(function(win) { Might it be clearer to use .some() instead of .every()? Or better yet just a plain old loop + break/continue? @@ +3049,5 @@ > + else > + dragPanel.removeAttribute("target"); > + > + if (!detached) { > + this.setAttribute("drag", "detach"); Set |detached = true| here? It's not used again, but lest someone add more code later... @@ +3062,5 @@ > + let preview = dragPanel.lastChild; > + label.value = draggedTab.label; > + label.width = preview.width = Math.min(outerWidth / 2, screen.availWidth / 5); > + let aspectRatio = this.tabbrowser.clientWidth / this.tabbrowser.clientHeight; > + preview.height = Math.min(preview.width / aspectRatio, screen.availHeight / 5); Do you want a Math.round() / Math.floor() here, to ensure the divisions are integers? @@ +3102,5 @@ > + .screenForRect(aLeft, aTop, 1, 1) > + .GetAvailRect(sX, sY, sWidth, sHeight); > + if (isPanel) { // Avoid getting repositioned by the window manager. > + sWidth.value -= 3; > + sHeight.value -= 3; Why 3? @@ +3254,5 @@ > + delete tab._maxWidth; > + delete tab._savedEvent; > + delete tab._targetWindow; > + delete tab._dropTarget; > + Hmm. Is this is to avoid leaking/bloat, and/or to make sure code doesn't thing we're mid-drag due to a stale property leftover from the last drag? There are enough of these that I wonder if it would be safer to stuff everything into a tab.dragData object. You would then just null it out here (or set it to a new, empty object), and if we add more bits to dragData later there's no risk of forgetting to clear them here. @@ +3526,4 @@ > break; > + let bo = this.mTabstrip.boxObject; > + if (aEvent.screenX >= bo.screenX && aEvent.screenX < bo.screenX + bo.width && > + aEvent.screenY >= bo.screenY && aEvent.screenY < bo.screenY + bo.height) Yet another of these! :)
Attachment #547614 -
Flags: review?(dolske) → review+
Comment 293•13 years ago
|
||
(In reply to comment #292) > > + background: -moz-element(#content) left top; > > I suppose this won't show the right thing when dragging a background tab? Yes, so I added back in a TabSelect handler, which Dolske rs'd. Add-ons will need to restyle the panel when the tab being dragged is not selected. This can be done using TabSelect and dragstart handlers that set an attribute on .tab-drag-panel and including something like the following in a stylesheet: .tab-drag-panel:not([selected]) { /* use something besides -moz-element */ } > > + var ltr = (window.getComputedStyle(this, null).direction == "ltr"); > > Would |ltr = (window.document.dir == "ltr");| work here? No, AFAICT, document.dir always returns "ltr" and does not necessarily reflect whether the tab strip is RTL. > > + delete draggedTab._dropTarget; > > Is this supposed to be part of the else-block? Seems odd to be nuking it > every call... It's okay to delete it, because we have to check what the value needs to be with every mousemove anyway. > Can any of the other similar "is x bounded by y" checks below make use of > this too? Especially if you passed in window.boxObject instead of window? No, window.boxObject is undefined. > > + if (!detached) { > > + this.setAttribute("drag", "detach"); > > Set |detached = true| here? It's not used again, but lest someone add more > code later... I don't think that's necessary or even useful. |detached| is more of a "wasDetached", and it's more likely that someone would want to know the previous value. > > + if (isPanel) { // Avoid getting repositioned by the window manager. > > + sWidth.value -= 3; > > + sHeight.value -= 3; > > Why 3? Empirical data. > > + delete tab._maxWidth; > > + delete tab._savedEvent; > > + delete tab._targetWindow; > > + delete tab._dropTarget; > > + > > Hmm. Is this is to avoid leaking/bloat, and/or to make sure code doesn't > thing we're mid-drag due to a stale property leftover from the last drag? The former. > There are enough of these that I wonder if it would be safer to stuff > everything into a tab.dragData object. Done. Carrying r+ forward. Will land soon. Additional followups will be filed shortly as well.
Attachment #547614 -
Attachment is obsolete: true
Comment 294•13 years ago
|
||
Pushed to m-c and also merged onto ux. https://hg.mozilla.org/mozilla-central/rev/9d4e815ae581 https://hg.mozilla.org/mozilla-central/rev/956bc17951c2 https://hg.mozilla.org/mozilla-central/rev/048afee8ae1e
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Target Milestone: --- → Firefox 8
Updated•13 years ago
|
Comment 295•13 years ago
|
||
Comment 296•13 years ago
|
||
I am not sure, but I see a little bit "heavier" animations when using Firefox in windowed mode. In maximized mode it's quite smooth. Probably I see difference because my CPU is slow (netbook Atom N450). I can't reproduce this on desktop machine (quad core). Win 7 Pro SP1 32bit, fresh profile, hourly build from http://hg.mozilla.org/mozilla-central/rev/e4c8a1e7b373
Comment 297•13 years ago
|
||
got this bug: http://img828.imageshack.us/img828/3423/bugnj.png detach tab...attach it again (over another tab)...repeat..bug happens got it with 2 tabs and with with many tabs also (5 tabs)
Comment 298•13 years ago
|
||
Fixed typo. https://hg.mozilla.org/mozilla-central/rev/d066929dd830
Comment 299•13 years ago
|
||
(In reply to comment #298) > Fixed typo. > > https://hg.mozilla.org/mozilla-central/rev/d066929dd830 So this looks like a code change. Why would it be marked DONTBUILD? Should not tests be re-run?
No longer depends on: 674732
Comment 300•13 years ago
|
||
Restoring dependency removed by previous comment/mid-air.
Depends on: 674732
Comment 301•13 years ago
|
||
Dragged tab seems to freeze and lag when alt-tabbing to a different window. Make a new Nightly window Go back to your currently used window Detach one tab alt-tab to the newly made window Try to move the tab to the tab area The dragged tab will freeze a bit below half the screen and be unresponsive, it needs to be dropped to make its own window to make any use of it. Here's a problem, alt-tabbing when the detached tab is close to the chrome poses no problem, it can be moved whenever I like, but if the detached tab is moved a bit below the chrome it becomes unresponsive after alt-tab. Should I make a new bug?
Comment 302•13 years ago
|
||
(In reply to comment #301) No. Bug 674487.
Comment 303•13 years ago
|
||
I have a little Problem with Windows 7: You cannot drag a detached Tab to the Task bar to bring a (hidden or minimized) firefox window up. In Windows 7 you can usually drag things over the icon in the taskbar to get minimized or hidden windows to the front.
Comment 304•13 years ago
|
||
First, woot for finally getting this bug landed. Kudos to fryn for sticking through it over the last year. ;-) Secondly, this bug is now closed. If you have problems or issues, please file NEW BUGS and mark them as blocking this one. There are 303 comments and 167 people CC'd to this bug -- further comments here just annoy people are are likely to get lost and not acted upon. Repeat: File NEW BUGS for any issues. We're done here. Thanks!
Comment 305•13 years ago
|
||
(In reply to comment #303) > I have a little Problem with Windows 7: You cannot drag a detached Tab to > the Task bar to bring a (hidden or minimized) firefox window up. > > In Windows 7 you can usually drag things over the icon in the taskbar to get > minimized or hidden windows to the front. Please file a bug on this.
Comment 306•13 years ago
|
||
Filed Bug 674831. We still need a bug about tabs dragged between windows not appearing as real tabs.
Comment 307•13 years ago
|
||
Comment on attachment 548845 [details]
Improving the design of preview the detached tab.
We do not necessarily know what the user's window caption buttons (min/max/close) look like, so this design is not necessarily more accurate nor more useful than the current one.
Attachment #548845 -
Attachment is obsolete: true
Updated•13 years ago
|
Comment 308•13 years ago
|
||
(In reply to comment #307) > Comment on attachment 548845 [details] > Improving the design of preview the detached tab. > > We do not necessarily know what the user's window caption buttons > (min/max/close) look like, so this design is not necessarily more accurate > nor more useful than the current one. Also user can have disabled app-button (:
Comment 309•13 years ago
|
||
This line removal got lost somehow. Text editors and I need to become better friends. https://hg.mozilla.org/mozilla-central/rev/fec9d1d409cf
Comment 310•13 years ago
|
||
Can we get an about:config setting to disable the animations? It breaks tab dragging in TMP and currently has conflicts with the task bar.
Comment 311•13 years ago
|
||
(In reply to comment #310) > Can we get an about:config setting to disable the animations? It breaks tab > dragging in TMP and currently has conflicts with the task bar. I've added bug 675451 which is aimed at that goal.
Comment 312•13 years ago
|
||
Hello, First, the tab animation is very good and the detach animation too. But I've found a small graphic bug in the detach animation : the bottom-right corner has not a correct shadow. Maybe, It happens only on my computer (W764bit, Intel Core 2 Duo, ASUS Nvidia 460 GTX).
Comment 313•13 years ago
|
||
Comment 314•13 years ago
|
||
(In reply to comment #313) > Created attachment 550222 [details] > A small bug of the detach animation. That would be Bug 458864.
Comment 315•13 years ago
|
||
Comment on attachment 550222 [details] A small bug of the detach animation. (In reply to comment #313) > Created attachment 550222 [details] > A small bug of the detach animation. Read comment 264, comment 265,and comment 304. Bug 458864 is a known issue.
Attachment #550222 -
Attachment is obsolete: true
Comment 316•13 years ago
|
||
After landing this bug, tabs can't be bookmarked by dropping them into the bookmarks toolbar. Now you need to press CTRL first. Is this new behavior set in stone or subject to discuss?
Comment 317•13 years ago
|
||
(In reply to comment #316) > After landing this bug, tabs can't be bookmarked by dropping them into the > bookmarks toolbar. Now you need to press CTRL first. Is this new behavior > set in stone or subject to discuss? Is it related to bug 674723?
Comment 318•13 years ago
|
||
(In reply to comment #317) > (In reply to comment #316) > > After landing this bug, tabs can't be bookmarked by dropping them into the > > bookmarks toolbar. Now you need to press CTRL first. Is this new behavior > > set in stone or subject to discuss? > > Is it related to bug 674723? Sorry. This comment is wrong.
Comment 319•13 years ago
|
||
this bug is FIXED, take your discussions to http://groups.google.com/group/mozilla.dev.apps.firefox/topics?lnk=rgh File follow up bugs blocking this one if you have problems/issues with the current patch/implementation. thanks
Comment 320•13 years ago
|
||
can I have the arrow indicator back? horrible 'animation'
Comment 321•13 years ago
|
||
(In reply to dindog from comment #320) > can I have the arrow indicator back? horrible 'animation' Watch bug 675451
Comment 322•13 years ago
|
||
I don't want this stinking animation. I want to be able to Drag and Drop my Tabs into the Bookmark Toolbar.
Comment 323•13 years ago
|
||
(In reply to Malcolm from comment #322) > I don't want this stinking animation. > > I want to be able to Drag and Drop my Tabs into the Bookmark Toolbar. Atleast try to read the previous comments , you are aware of the fact that your repetitive comment has bothered around 150 people . And for your answer : You can drag your tab to the bookmarks toolbar by holding Ctrl .
Comment 324•13 years ago
|
||
What's that Firefox Slogan advertisement on the Billboards: "Every browser does fast. But not every browser does good." After Firefox 4, there is just an added hassle to get the features I want. First I had to go to Options and enable the Menu Bar. After that I had to go to View ==> Toolbars and enable the Bookmarks Toolbar. Then I had to Uncheck ==> Tabs On Top. Then I had to click View ==> Toolbars ==> Customize to get the Home Screen & Stop Button in the proper location. And then Firefox changed the location of the "Open in New Tab" when we do right click. Instead of opening in new tab I kept opening in new window (like IE6). I finally got used to it now. Coming to that topic why did you not remove "Open in New Window" while you went around gutting all the features for Firefox 4 and making it so much harder to use Firefox. And now coming soon to Firefox 9 ==> More hassles, starting with pressing CTRL when bookmarking a tab, and who knows what else the UI teams wants to break. Plus new UI means more bloat, regression bugs, and also I am sure more security vulnerabilities. Lets change this slogan to: "Every browser does fast. No browser does good."
Comment 325•13 years ago
|
||
(In reply to Malcolm from comment #324) What in the world does any of this have to do with the tab reorder animation???? Please don't spam bugs.
Comment 326•13 years ago
|
||
Whatever you think you're trying to accomplish by ranting in a closed bug, Malcolm, you're not accomplishing it. You are, however, emailing 163 people every time you try. If you have constructive suggestions for changes, and can express them without insulting the people who try very hard to build a good browser, please file new bugs.
Comment 327•13 years ago
|
||
from Girish Sharma scrapmachines@gmail.com to malcolm.turmel@gmail.com date Tue, Aug 23, 2011 at 7:40 AM subject Re : [Bug 455694] Implement animation for tab reordering and detaching (via drag and drop) hide details 7:40 AM (5 minutes ago) (In reply to Malcolm from comment #324) > What's that Firefox Slogan advertisement on the Billboards: > > "Every browser does fast. But not every browser does good." > > After Firefox 4, there is just an added hassle to get the features I want. > > First I had to go to Options and enable the Menu Bar. > > After that I had to go to View ==> Toolbars and enable the Bookmarks Toolbar. > > Then I had to Uncheck ==> Tabs On Top. > > Then I had to click View ==> Toolbars ==> Customize to get the Home Screen & > Stop Button in the proper location. > > And then Firefox changed the location of the "Open in New Tab" when we do > right click. Instead of opening in new tab I kept opening in new window > (like IE6). I finally got used to it now. > > Coming to that topic why did you not remove "Open in New Window" while you > went around gutting all the features for Firefox 4 and making it so much > harder to use Firefox. > > And now coming soon to Firefox 9 ==> More hassles, starting with pressing > CTRL when bookmarking a tab, and who knows what else the UI teams wants to > break. > > Plus new UI means more bloat, regression bugs, and also I am sure more > security vulnerabilities. > > Lets change this slogan to: > > "Every browser does fast. No browser does good." Ohk , we get it , you are that lazy bum who can't even cope up with the slow changes of the world, and who just wants things the old retarded way, smoking out his pipe , laying back on drugs , and just liking the browser as it was ages ago , howsoever new and better technology arrives , he just wants his **** old **** ways to carry on. I think you get my pint . Also , please don't spam that bug anymore. Have mercy on other 163 persons following that bug. Stop venting you **** frustration here on this bug. -- Girish Sharma Final Year Student Civil Engineering Department IIT Kharagpur
Comment 328•13 years ago
|
||
srsly, we are not interested in your internet social relations... Please go to mozilla.dev.usability newsgroup to discuss your product usability concerns and leave your feedback in a constructive way.
Comment 329•13 years ago
|
||
Bug 681368 filed for ctrl-click change.
Updated•13 years ago
|
Comment 330•13 years ago
|
||
Malcolm's account has been disabled. That sort of abuse is not acceptable in Bugzilla. Please report it if you see it. Gerv
Comment 331•13 years ago
|
||
For tabs which have a PDF opened (using a plugin) no preview appears in the dragged out widget. No preview appears in Panorama also. Is there a tracking bug for the missing previews?
Updated•13 years ago
|
Whiteboard: [parity-safari][parity-chrome][parity-ie] → [parity-safari][parity-chrome][parity-ie][qa+]
Updated•13 years ago
|
Comment 332•13 years ago
|
||
verified [testday-20110930]
Comment 333•13 years ago
|
||
verified Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0) Gecko/20100101 Firefox/8.0
Comment 334•13 years ago
|
||
verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0) Gecko/20100101 Firefox/8.0b1
Comment 335•13 years ago
|
||
Verified fixed also with 8.0b1 Linux x86_64 eo.
Status: RESOLVED → VERIFIED
Whiteboard: [parity-safari][parity-chrome][parity-ie][qa+] → [parity-safari][parity-chrome][parity-ie][qa+][testday-2011030]
Updated•13 years ago
|
Whiteboard: [parity-safari][parity-chrome][parity-ie][qa+][testday-2011030] → [parity-safari][parity-chrome][parity-ie][qa+][testday-20110930]
Comment 336•13 years ago
|
||
Verified also using the latest Nightly, the latest Aurora and Firefox 8.0b1 on Windows XP, Windows 7, Ubuntu 11.04 and Mac OS X 10.6. Animation for tab reordering and detaching is implemented as in the prototype from Comment 12 with the exception that the tabs can't be dragged past the left and right edges of the screen (as mentioned in Comment 99). In the prototype from Comment 12 beside Move and Detach tabs there are 2 more tabs: Close and New Tab. Are Close and New Tab supposed to be implemented by this path also?
Comment 337•13 years ago
|
||
Was this a partial back-out ? https://hg.mozilla.org/mozilla-central/rev/4bb7c983d589 should it be re-opened ?
Comment 338•13 years ago
|
||
It was not backed out of mozilla-central so it should not be reopened yet. Once it is, this bug is unfixed and needs to be reopened.
Comment 339•13 years ago
|
||
(In reply to Christian Legnitto [:LegNeato] from comment #338) > It was not backed out of mozilla-central so it should not be reopened yet. > Once it is, this bug is unfixed and needs to be reopened. then I'm confused - the cset in comment #337 clearly shows m-c ?
Comment 340•13 years ago
|
||
Yep, my bad, missed the m-c part.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Comment 341•13 years ago
|
||
Yes, we backed it out. I don't think we should keep this bug reopened. The thread is long enough. In the past, we've not reopened many bugs where things were landed, effectively backed out, and re-landed later in a vastly different form. Either way, I'm not going to do development in this bug. Development of the new, better implementation will happen in bug 674925, and this bug can be re-closed when the patch there lands.
Status: REOPENED → UNCONFIRMED
Ever confirmed: false
Whiteboard: [parity-safari][parity-chrome][parity-ie][qa+][testday-20110930] → [backed out in bug 690227 -- see bug 674925][parity-safari][parity-chrome][parity-ie][qa+][testday-20110930]
Comment 342•13 years ago
|
||
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 343•13 years ago
|
||
Probably best to dupe this over to the new development bug. Keeping this open as new if it won't see a patch is pointless.
Comment 344•13 years ago
|
||
(In reply to Paul [sabret00the] from comment #343) > Probably best to dupe this over to the new development bug. Keeping this > open as new if it won't see a patch is pointless. Which is?
Comment 345•13 years ago
|
||
Yeah, we're done here. This specific implementation has been backed out, and will be reimplemented over in bug 674925 as Frank notes in comment 341. There are enough people CC'd here and related bugs that I'm not to dupe this over. Feel free to CC yourself to bug 674925 if you're interested. WONTFIX for this specific implementation.
Status: NEW → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → WONTFIX
Comment 346•13 years ago
|
||
What is it? Like a year of rather active work on it? And now dropped. No wonder firefox keeps falling behind.
Comment 347•13 years ago
|
||
(In reply to avada from comment #346) > What is it? Like a year of rather active work on it? And now dropped. No > wonder firefox keeps falling behind. There are problems with this form of implementation with almost All tab-related add-ons. For more info, see Bug 690227. Hence, it is going to be reimplemented in bug 674925.
Comment 348•13 years ago
|
||
(In reply to Justin Dolske [:Dolske] from comment #345) > There are enough people CC'd here and related bugs that I'm not to dupe > this over. Feel free to CC yourself to bug 674925 if you're interested. Marking a bug as a duplicate doesn't copy the CC list, unless this changed recently, which I hope isn't the case...
Resolution: WONTFIX → DUPLICATE
Updated•12 years ago
|
Keywords: uiwanted
Whiteboard: [backed out in bug 690227 -- see bug 674925][parity-safari][parity-chrome][parity-ie][qa+][testday-20110930] → [backed out in bug 690227 -- see bug 674925][parity-safari][parity-chrome][parity-ie]
Updated•11 years ago
|
Assignee: fyan → nobody
You need to log in
before you can comment on or make changes to this bug.
Description
•