New Tab icon doesn't get put next to tabs when inserted after the (CSS-moved) titlebar placeholder

RESOLVED FIXED in Firefox 61

Status

()

defect
P3
normal
RESOLVED FIXED
Last year
8 months ago

People

(Reporter: tom.mai78101, Assigned: Gijs)

Tracking

60 Branch
Firefox 61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 wontfix, firefox60 wontfix, firefox61 fixed)

Details

Attachments

(4 attachments)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.125 Safari/537.36

Steps to reproduce:

1: Swap the "+" button with the chevron button when Customizing the UI.
2: Validate
3: Swap back

Closing Dev Edition and reopening it seems to "fix it".





Actual results:

As shown in the GIF, the "+" button is moved to the far left or far right, never at the end of the right-most tab.


Expected results:

The "+" button should be next to the right-most tab, where it should be in the previous version of Firefox.
Component: Untriaged → Developer Tools
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64
Component: Developer Tools → Theme
Component: Theme → Toolbars and Customization
I've looked at both of the gifs several times and I don't understand at what point you think the behavior is wrong, or what you think is different compared to previous versions. comment #0 also doesn't really help:

(In reply to tom.mai78101 from comment #0)
> 1: Swap the "+" button with the chevron button when Customizing the UI.
> 2: Validate

??? What does this mean?

> 3: Swap back

I assume this means swap the chevron and the [+] button? 


> Closing Dev Edition and reopening it seems to "fix it".

Fix what?

> Actual results:
> 
> As shown in the GIF, the "+" button is moved to the far left or far right,
> never at the end of the right-most tab.

Only while keeping customize mode open, right? That has never worked, I don't think. At least, I don't see it working in 56...

If you keep the item right next to the tabstrip on the right-hand-side (ie not with the chevron inbetween), it'll snap to the end of the right-most tab when you exit customize mode.

Please can you clarify specifically what configuration you think doesn't work right, and when, and in what Firefox version this apparently worked differently?
Flags: needinfo?(tom.mai78101)
If this is supposed to be about where the button appears while customize mode is open, I think that's covered in (probably among others) bug 581759.
Hi, I'll try to explain.

(In reply to :Gijs (he/him) from comment #2)
> (In reply to tom.mai78101 from comment #0)
> > 1: Swap the "+" button with the chevron button when Customizing the UI.
> > 2: Validate
> 
> ??? What does this mean?

The button in GIF 1 that I'm dragging is the + button, which is the Add New Tab button. Normally, the placement of the Add New Tab button is located here:

https://i.imgur.com/ZcWuw87.png

(Sorry for the background, currently the Add New Tabs button is too light for light backgrounds)

And when you drag the Add New Tab button around in Customize Mode, there should at least be one valid placement where it can be placed next to the rightmost active tab (where the picture is pointing at). But that placement is considered invalid, or it doesn't exist in Customize Mode, so you see me trying to attempt to put the Add New Tab button to that location in GIF 1, and failing to do so.

> 
> > 3: Swap back
> 
> I assume this means swap the chevron and the [+] button? 

Yes, swapping as in, switching the placement button of the ^ button and the + button around in Customize Mode. All buttons react the same way, where they all can't find the placement next to the rightmost active tabs (again, the Imgur picture above).

> 
> 
> > Closing Dev Edition and reopening it seems to "fix it".
> 
> Fix what?

Fix the placement issue experienced in GIF 2. The Add New Tab button gets reset back to the intended placement, hence it "seemed to have fixed the issue", by reinvalidating the UI Theme layout.


If you still need clarification, let me know.
Flags: needinfo?(tom.mai78101)
(In reply to tom.mai78101 from comment #4)
> And when you drag the Add New Tab button around in Customize Mode, there
> should at least be one valid placement where it can be placed next to the
> rightmost active tab (where the picture is pointing at). But that placement
> is considered invalid, or it doesn't exist in Customize Mode, so you see me
> trying to attempt to put the Add New Tab button to that location in GIF 1,
> and failing to do so.

Right. In customize mode the tabstrip takes up as much of the width of the toolbar as it can (so, all of it, minus whatever other buttons are in that toolbar, which by default is the chevron and the [+] button) Here "tabstrip" includes not just the 2 tabs but also all the empty space to their right. The standalone button can't be "inside" the tabstrip, which is why it shows up all the way to its right. Just closing customize mode by clicking "Done" at the bottom (or hitting escape, or switching to another tab) will snap the button back if there are no other buttons between the [+] and the tab.

Allowing the tab button to show up in the same place it normally shows up is bug 581759 ("Make the add tab button appear next to the last normal tab in customize mode"). It's difficult to fix because you can't drag the tabstrip, and you would still want to be able to drag the [+] button, which would be "inside" the tabstrip element in this case... We can't just make the tabstrip smaller because all the *other* buttons won't line up with the tabs outside customize mode, so that would mean *those* buttons would be "misplaced" compared to where they are in customize mode.

> Fix the placement issue experienced in GIF 2. The Add New Tab button gets
> reset back to the intended placement, hence it "seemed to have fixed the
> issue", by reinvalidating the UI Theme layout.

Right, but it'll do that (ie snap back) even if you just exit customize mode... So if you follow something like these steps:

1. go into customize mode
2. swap the [+] and chevron around
3. go out of customize mode. Now the [+] is all the way at the right edge of the toolbar.
4. go back into customize mode.
5. swap the buttons back around.
6. exit customize mode. Now the [+] is next to the tabs again.

Things more or less work (ie the [+] is where you expect it to be).

Do you not see the button move next to the tabs immediately once you exit customize mode by clicking "Done", iff there are no other buttons between the tabs and the [+] ? To me, it seems like that's what's happening at the end of gif 2...
Flags: needinfo?(tom.mai78101)
Well, now I know what is going on, I'll have to get back to you on Monday in order to test and see what's going on, since I'm not at the computer that I tested and filed the issue from.

Sorry for the inconvenience.
(In reply to tom.mai78101 from comment #6)
> Well, now I know what is going on, I'll have to get back to you on Monday in
> order to test and see what's going on, since I'm not at the computer that I
> tested and filed the issue from.
> 
> Sorry for the inconvenience.

Hi Tom, any news here on what's going on? :-)
Sorry for the delay, this week's had been hectic at work.

I have added a longer GIF that shows that it's still possible to break the new tab button.
Flags: needinfo?(tom.mai78101)
Attachment #8970949 - Attachment description: GIF.gif → Firefox_feat_newTabButton.gif
So, after multiple times swapping the buttons back and forth, and placing the new tab button in different places, I was expecting the new tab button would be placed correctly every time the new tab button is placed on the immediate right side of the tabs. But, then I started to notice the button stopped being placed next to the tabs after moving it out and into some other places.

Been trying to re-align the button after entering and exiting Customize Mode, but I don't know how to do so unless I exit/reopen Firefox.
Oof. Yeah, I can reproduce with these details, thanks for the clarification. The confusion is because the new tab button gets inserted after the titlebar button placeholder element, and when we look for the next sibling of the tabstrip we find those first (we use CSS to reposition them to the end of the tabstrip / titlebar). That, at least, is easy to fix...
Assignee: nobody → gijskruitbosch+bugs
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P3
Summary: New Tab icon position placement is in the wrong spot → New Tab icon doesn't get put next to tabs when inserted after the (CSS-moved) titlebar placeholder
Blocks: 1370791
OS: Windows 7 → All
Hardware: x86_64 → All
Comment on attachment 8970958 [details]
Bug 1445728 - fix new tab button adjacency logic to deal with titlebar placeholders and/or indicators appearing in the middle,

https://reviewboard.mozilla.org/r/239712/#review246256

This looks good now, thank you!

::: browser/base/content/tabbrowser.xml:872
(Diff revision 2)
>            let sib = this;
>            do {
>              sib = unwrap(wrap(sib).nextElementSibling);
> -          } while (sib && sib.hidden);
> +          } while (sib && (sib.hidden ||
> +                           sib.getAttribute("skipintoolbarset") == "true" ||
> +                           sib.id == "alltabs-button"));

I'd say this line deserves a comment as to why we're skipping that button (I wouldn't understand it without the context of this bug).
Attachment #8970958 - Flags: review?(jhofmann) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/dc916a5c4dc8
fix new tab button adjacency logic to deal with titlebar placeholders and/or indicators appearing in the middle, r=johannh
https://hg.mozilla.org/mozilla-central/rev/dc916a5c4dc8
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Thanks, everyone.
Blocks: 1513200
You need to log in before you can comment on or make changes to this bug.