The default bug view has changed. See this FAQ.

Clicking on active tab doesn't bring it back into view (close button hidden) when scroll arrows were used and tab is halfway hidden

RESOLVED FIXED in Firefox 53

Status

()

Firefox
Tabbed Browser
--
minor
RESOLVED FIXED
8 years ago
16 days ago

People

(Reporter: thatiparthysreenivas, Assigned: Premang)

Tracking

3.5 Branch
Firefox 53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [FFT3.5])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1) Gecko/20090616 Firefox/3.5 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1) Gecko/20090616 Firefox/3.5 (.NET CLR 3.5.30729)

Open as many tabs as possible so that you can see left scroll arrow and right scroll  arrow buttons.Now select any tab,click on the left scroll to send selected  tab to right and it should be half way through the right scroll ,so that u cant see close(x) mark,if you click again on the selected tab ,you don't get the close(x) mark to close that particular tab.ex: Open 11 tabs,select 2 nd one from right most and click on left scroll arrow to make it half way through of right scroll arrow.Then click again on the selected tab.You will get behaviour as explained above.You can  reproduce this for no of tabs >11 situation.

Reproducible: Always

Steps to Reproduce:
1.Open as many tabs as possible so that you can see left scroll arrow and right scroll  arrow buttons( no of tabs >11 )
2.Now select any tab,click on the left scroll to send selected  tab to right and it should be half way through the right scroll ,so that u cant see close(x) mark.
3.you click again on the selected tab
Actual Results:  
the selected tab is not scrolling left to show the close(x) mark.

Expected Results:  
the selected tab should be scrolled left to show the close(x) mark

No themes installed .
(Reporter)

Updated

8 years ago
Summary: selected tab is not scrolled back when it selected again to see close(x) button to close it, while it is halfway through right arrow. → selected tab is not scrolled back when it is selected again to see close(x) button to close it, while it is halfway through right arrow.
(Reporter)

Updated

8 years ago
Summary: selected tab is not scrolled back when it is selected again to see close(x) button to close it, while it is halfway through right arrow. → selected tab is not scrolled back when it is selected again to see close(x) button to close it, while it is halfway through right scroll arrow.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All
Summary: selected tab is not scrolled back when it is selected again to see close(x) button to close it, while it is halfway through right scroll arrow. → Clicking on active tab doesn't bring it back into view (close button hidden) when scroll arrows were used and tab is halfway hidden
Whiteboard: [FFT3.5]
Version: unspecified → 3.5 Branch
Created attachment 383892 [details]
screenshot

Resizing the window for a couple of pixels solves it.
(Assignee)

Comment 2

3 months ago
I would like to work on this Bug, can I please?
(Assignee)

Updated

3 months ago
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 3

3 months ago
Is it ok to scroll to right when user hover the mouse pointer on partially visible tab, in order to make the tab fully visible?
Flags: needinfo?(dao+bmo)
(Assignee)

Updated

3 months ago
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 4

3 months ago
Created attachment 8825179 [details] [diff] [review]
changes.patch

I've done changes to reflect the idea I proposed. Please review it.
Attachment #8825179 - Flags: review?(dao+bmo)

Comment 5

3 months ago
Comment on attachment 8825179 [details] [diff] [review]
changes.patch

Need to get ui-review first.
Flags: needinfo?(dao+bmo)
Attachment #8825179 - Flags: review?(dao+bmo) → ui-review?(ux-review)

Comment 6

3 months ago
Comment on attachment 8825179 [details] [diff] [review]
changes.patch

Try builds will appear here:
https://archive.mozilla.org/pub/firefox/try-builds/dgottwald@mozilla.com-3c09844c41576c13ceee4dacd2da12f3a318cd03/
Attachment #8825179 - Flags: ui-review?(ux-review) → ui-review?(mverdi)

Comment 7

2 months ago
Comment on attachment 8825179 [details] [diff] [review]
changes.patch

This works well and will be a nice improvement. The one change we should make though is to make the new animation match the other tab animation. I'll attach a video that shows the difference but basically this new animation is more of a pop where the animation when you click on a tab that is half-visible but not active has a bit of easing to it. I assume the values for the animation for that code can be used here.
Attachment #8825179 - Flags: ui-review?(mverdi) → ui-review-

Comment 8

2 months ago
Created attachment 8827444 [details]
tabanimation.mp4

The first part of this video shows the new interaction where the tab pops into place. After the fade to black is an example of the animation for a similar interaction. There the tab kind of slides into place.

Comment 9

2 months ago
Comment on attachment 8825179 [details] [diff] [review]
changes.patch

>+          if(this.selected)

nit: space after 'if'

>+            tabContainer._handleTabSelect(false);

Calling _handleTabSelect without 'false' as the argument should enable the animation that Verdi was talking about.

Would you like to update the patch?
Flags: needinfo?(premangvikani)
(Assignee)

Comment 10

2 months ago
(In reply to Dão Gottwald [:dao] from comment #9)
> Comment on attachment 8825179 [details] [diff] [review]
> changes.patch
> 
> >+          if(this.selected)
> 
> nit: space after 'if'
> 
> >+            tabContainer._handleTabSelect(false);
> 
> Calling _handleTabSelect without 'false' as the argument should enable the
> animation that Verdi was talking about.
> 
> Would you like to update the patch?

Yes I would like to update..
(Assignee)

Comment 11

2 months ago
Comment on attachment 8825179 [details] [diff] [review]
changes.patch

>diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml
>--- a/browser/base/content/tabbrowser.xml
>+++ b/browser/base/content/tabbrowser.xml
>@@ -7011,6 +7011,10 @@
>           let tabContainer = this.parentNode;
>           let visibleTabs = tabContainer.tabbrowser.visibleTabs;
>           let tabIndex = visibleTabs.indexOf(this);
>+
>+          if (this.selected)
>+            tabContainer._handleTabSelect();
>+          
>           if (tabIndex == 0) {
>             tabContainer._beforeHoveredTab = null;
>           } else {
Attachment #8825179 - Flags: review?(dao+bmo)
(Assignee)

Comment 12

2 months ago
Created attachment 8827753 [details] [diff] [review]
Edited patch as suggested.

I edited the patch as you suggested and now it shows the animation as wanted.
Attachment #8827753 - Flags: review?(dao+bmo)

Updated

2 months ago
Attachment #8825179 - Attachment is obsolete: true
Attachment #8825179 - Flags: review?(dao+bmo)
Comment on attachment 8827753 [details] [diff] [review]
Edited patch as suggested.

>diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml
>--- a/browser/base/content/tabbrowser.xml
>+++ b/browser/base/content/tabbrowser.xml
>@@ -7011,6 +7011,10 @@
>           let tabContainer = this.parentNode;
>           let visibleTabs = tabContainer.tabbrowser.visibleTabs;
>           let tabIndex = visibleTabs.indexOf(this);
>+
>+          if (this.selected)
>+            tabContainer._handleTabSelect();
>+          

Looks good, but the seemingly empty line at the bottom has a bunch of trailing spaces. Please remove these.

Thanks!
Attachment #8827753 - Flags: review?(dao+bmo) → review+
(Assignee)

Comment 14

2 months ago
Created attachment 8827970 [details] [diff] [review]
Final patch

Extra spaces removed.
Flags: needinfo?(premangvikani)
Attachment #8827970 - Flags: review?(dao+bmo)

Updated

2 months ago
Attachment #8827753 - Attachment is obsolete: true
Comment on attachment 8827970 [details] [diff] [review]
Final patch

Thanks!
Attachment #8827970 - Flags: review?(dao+bmo) → review+

Updated

2 months ago
Assignee: nobody → premangvikani
(Assignee)

Comment 16

2 months ago
Dao, can you please do try build for the patch? And if not required then please tell what am I supposed to do next for this bug? Should I add checkin-needed keyword? or something else?
Flags: needinfo?(dao+bmo)
(In reply to Premang from comment #16)
> Should I add checkin-needed keyword? or something else?

Yes. I would have landed this earlier if the tree hadn't been closed.
Flags: needinfo?(dao+bmo)
(Assignee)

Updated

2 months ago
Keywords: checkin-needed

Comment 18

2 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/02c94a326f6d
Scroll the selected tab into view on mouseover when it's only partially visible. r=dao
Keywords: checkin-needed

Comment 19

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/02c94a326f6d
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.