Closed Bug 860648 Opened 11 years ago Closed 11 years ago

Entering/exiting customization mode via switching to/from about:customizing can result in detaching a tab

Categories

(Firefox :: Toolbars and Customization, defect)

x86_64
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: Unfocused, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:M9][Australis:P4])

Attachments

(1 file, 2 obsolete files)

Fallout from bug 858597, which made switching tabs to/from about:customizing enter/exit customization mode. Because the tab-strip moves when entering/exiting customization mode, if you switch tabs by using your mouse and hold down the mouse button for just a little too long, you can end up accidentally dragging the tab - which then gets detached.
I'm not sure we can actually fix this with the current behaviour :\ 

But in general, being able to switch back and forth between about:customizing and any other tab feels really weird to me, since it changes everything about the whole window. So one solution would be to disable interacting with any other tabs, and just allow closing about:customizing (which IIRC was the old original plan).

Thoughts, Madhava?
Flags: needinfo?(madhava)
No longer blocks: 770135
Whiteboard: [Australis:M7]
I think this is a legitimate concern. It might not block landing per se, but it might affect perception. I suspect this case might also be easier to trigger on less powerful machines.
Assignee: nobody → mconley
OS: Windows 8 → All
Taking this off the M7 blocker list for now.
Whiteboard: [Australis:M7] → [Australis:M?]
Whiteboard: [Australis:M?] → [Australis:M?][Australis:P4]
Status: NEW → ASSIGNED
So this fixes the issue, as well as the unintentional variant of bug 890319 and the weirdness in bug 879313. All at the cost of tab dnd while in customize mode. I *think* that's OK. Maybe people feel differently, though... in a more elaborate ploy, we could try to store something on the drag transfer data and only refuse to detach tabs if you started dragging them in customize mode.
Attachment #819747 - Flags: review?(mconley)
Assignee: mconley → gijskruitbosch+bugs
Comment on attachment 819747 [details] [diff] [review]
shouldn't be able to dnd tabs in customize mode,

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

Hey Gijs - this looks OK to me, but what do you say about my idea for reducing duplication?

::: browser/base/content/tabbrowser.xml
@@ +4252,5 @@
>        <handler event="dragstart"><![CDATA[
>          var tab = this._getDragTargetTab(event);
>          if (!tab)
>            return;
> +        let isCustomizing = document.documentElement.getAttribute("customizing") == "true" ||

Perhaps it'd be better to move this logic into a private, read-only field for the tabbrowser binding? That way we can avoid doing this twice.
Attachment #819747 - Flags: review?(mconley)
(In reply to :Gijs Kruitbosch from comment #4)
> So this fixes the issue, as well as the unintentional variant of bug 890319
> and the weirdness in bug 879313. All at the cost of tab dnd while in
> customize mode. I *think* that's OK.

+1 - I think this is a good trade-off, and as an added benefit simplifies the possible interactions quite a bit (we're in the mode to customize widgets - tabs are not widgets).
Flags: needinfo?(madhava)
Sounds good to me!
Attachment #820247 - Flags: review?(mconley)
Attachment #819747 - Attachment is obsolete: true
Sorry, that was sloppy - coalesced the dragstart condition.
Attachment #820248 - Flags: review?(mconley)
Attachment #820247 - Attachment is obsolete: true
Attachment #820247 - Flags: review?(mconley)
Comment on attachment 820248 [details] [diff] [review]
shouldn't be able to dnd tabs in customize mode,

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

Yeah, this is good. My brain kinda tweaks looking at "this are customizing", so I *think* I'd prefer "_isCustomizing" instead if you have the time to change it, but it's not too big a deal.

Thanks Gijs!
Attachment #820248 - Flags: review?(mconley) → review+
https://hg.mozilla.org/projects/ux/rev/a5e5a65de58a
Whiteboard: [Australis:M?][Australis:P4] → [Australis:M9][Australis:P4][fixed-in-ux]
https://hg.mozilla.org/mozilla-central/rev/a5e5a65de58a
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M9][Australis:P4][fixed-in-ux] → [Australis:M9][Australis:P4]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: