Cannot insert customizable items to the left of the tabstrip

RESOLVED FIXED in Firefox 28

Status

()

Firefox
Toolbars and Customization
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: curtisk, Assigned: Gijs)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 28
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M6])

Attachments

(3 attachments, 1 obsolete attachment)

Latest UX nightly
If you move the home button from it's default location in the the upper left corner it can not be returned to that location. The restore defaults also does not return it to this location.
Hey Curtis - can you show us a screenshot for where you've moved the home button?
Flags: needinfo?(curtisk)
Curtis, are you still experiencing this? Can you give me a screenshot?
No longer blocks: 869104
Whiteboard: [Australis:M6]
Created attachment 749969 [details]
old home
Flags: needinfo?(curtisk)
Attached 2 screen shots both of the same profile one with current nightly and one with nightlyux
old home shows the home button where I've had it for ages
Aus home shows where it is now and I can't move it to the left side like I have it on all other FX browswer profiles I use
Ok, I see the problem now. Thank you Curtis.
Summary: Home button can not be returned to default location → Cannot insert customizable items to the left of the tabstrip
(Assignee)

Updated

5 years ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
(Assignee)

Comment 7

5 years ago
Current guess: we're not wrapping the <tabs> element in a toolbarpaletteitem wrapper, so no droptarget, so no dropping it on (and therefore left) of the tabs.
(Assignee)

Comment 9

5 years ago
(In reply to Mike Conley (:mconley) from comment #8)
> I think that's highly likely.

See, that's what I thought, and a couple of minutes debugging further and I'm confused. We're not even getting a drop event. Nor is the tabbrowser.xml code. Something's swallowing it, but I have no idea what it is. :s
(Assignee)

Comment 10

5 years ago
(In reply to :Gijs Kruitbosch from comment #9)
> (In reply to Mike Conley (:mconley) from comment #8)
> > I think that's highly likely.
> 
> See, that's what I thought, and a couple of minutes debugging further and
> I'm confused. We're not even getting a drop event. Nor is the tabbrowser.xml
> code. Something's swallowing it, but I have no idea what it is. :s

OK, using capturing event listeners fixes this. Probably a good idea anyway. However, now I have the problem I was originally afraid of - not a toolbarpaletteitem, things don't go well. :-)
(Assignee)

Comment 11

5 years ago
Created attachment 754745 [details] [diff] [review]
Patch v1

This turned out to be trickier than I'd hoped. 3 things change with this patch:

1. All the event listeners go capturing. I think this is a saner choice generally, so I executed it everywhere.
1b. Use the fact that that gives us customizableArea === event.currentTarget (fewer parentNode walking to figure out which area we're in), move actual node figurings-out down in the functions so we do the more expensive stuff less often
1c. For the target node, don't just blindly take what the dnd bits give us, but look for something which is an actual child of the drop area, so the position-figuring-out code doesn't screw up.
2. unwrapToolbarItem... is nice, but shouldn't try to unwrap things which aren't a toolbarpaletteitem. Leads to force-closing browser windows (!?) when the <tabs id="tabbrowser-tabs"> element starts disappearing and stuff... yeah, not great.
3. Make the target (drop) node ID getting a bit more flexible. Not everything need be wrapped in a toolbarpaletteitem. If it's not, though, it should be a direct child of the toolbar (which we ensured in 1c).
Attachment #754745 - Flags: review?(mconley)
Comment on attachment 754745 [details] [diff] [review]
Patch v1

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

Some behavioural problems with this patch:

1) Dragging items into the nav-bar breaks. The item doesn't show in the nav-bar, and the dragged item is unwrapped and left in its origin area
2) I dragged an item to the left of the tabstrip, and then the tabstrip was draggable. Not ideal.
Attachment #754745 - Flags: review?(mconley) → review-
(Assignee)

Comment 13

5 years ago
Created attachment 755278 [details] [diff] [review]
Patch v1.1

This should fix those two issues, at least.

I've also added styling so you get a dropmarker for inserting before the tabstrip (woo)

I also experimented briefly with just using a <toolbaritem removable="false"> around the tabbrowser-tabs. That essentially broke the world in all manner of exciting ways (like the tabs disappearing except in customize mode?!), so I went back to this patch. The issues you identified were relatively trivial to fix anyhow, but I noticed that the tabs are essentially the only item which isn't considered customizable by the code...
Attachment #754745 - Attachment is obsolete: true
Attachment #755278 - Flags: review?(mconley)
Comment on attachment 755278 [details] [diff] [review]
Patch v1.1

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

I'm good with this - just one suggestion below. Thanks Gijs!

::: browser/themes/shared/customizableui/customizeMode.inc.css
@@ +108,5 @@
> +  border-left: 2px solid transparent;
> +  border-right: 2px solid transparent;
> +}
> +
> +toolbar[customizing="true"] > tabs[dragover="left"] {

Let's not put these in actually, since they'll need to go away anyways after bug 873056 lands.
Attachment #755278 - Flags: review?(mconley) → review+
(Assignee)

Comment 15

5 years ago
Pushed: https://hg.mozilla.org/projects/ux/rev/e66284b3e110

(without the style bits)
Whiteboard: [Australis:M6] → [Australis:M6][fixed-in-ux]
(Assignee)

Comment 16

5 years ago
(In reply to :Gijs Kruitbosch from comment #15)
> Pushed: https://hg.mozilla.org/projects/ux/rev/e66284b3e110
> 
> (without the style bits)

Except I screwed up and didn't resolve the conflicts caused by our fancy new drag placeholders correctly. Fixed by https://hg.mozilla.org/projects/ux/rev/3c4799dd127c .
(Assignee)

Comment 17

5 years ago
https://hg.mozilla.org/mozilla-central/rev/e66284b3e110
https://hg.mozilla.org/mozilla-central/rev/3c4799dd127c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M6][fixed-in-ux] → [Australis:M6]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.