Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Changing tabset count messes up tab favicon and close button #9779

Closed
srirambv opened this issue Jun 29, 2017 · 12 comments
Closed

Changing tabset count messes up tab favicon and close button #9779

srirambv opened this issue Jun 29, 2017 · 12 comments

Comments

@srirambv
Copy link
Collaborator

srirambv commented Jun 29, 2017

Test plan

#10134 (comment)


  • Did you search for similar issues before submitting this one?
    Yes

  • Describe the issue you encountered:
    Changing tabset count messes up tab favicon and close button

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Windows 10 x64

  • Brave Version (revision SHA):
    Brave 0.17.12
    rev 0095c35
    Muon 4.1.6

  • Steps to reproduce:

    1. Set tab count to 20 and set homepage to reddit.com/r/random
    2. Open more than 100 tabs by pressing ctrl+t, ensure there are 5-6 tabset
    3. With all these tabs set tabcount to 100, close button and favicon on tabs gets messed up as the breakpoint is not properly calculated
  • Actual result:
    Changing tabset count messes up tab favicon and close button

  • Expected result:
    Tab breakpoint should be properly calculated

  • Will the steps above reproduce in a fresh profile? If not what other info can be added?
    Yes

  • Is this an issue in the currently released version?
    Yes

  • Can this issue be consistently reproduced?
    Yes

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:
    image

  • Any related issues:
    cc: @cezaraugusto

@srirambv srirambv modified the milestones: Backlog, 1.2.0 Jun 29, 2017
@cezaraugusto cezaraugusto self-assigned this Jun 29, 2017
@srirambv srirambv modified the milestones: 0.18.x (Developer Channel), 1.2.0 Jun 29, 2017
@cezaraugusto
Copy link
Contributor

dooh me they were the same, thanks Sri

@bbondy bbondy modified the milestones: 0.17.14 (Release Channel), 0.18.x (Beta Channel) Jul 8, 2017
@cezaraugusto cezaraugusto modified the milestones: 0.18.x (Beta Channel), 0.17.14 (Release Channel) Jul 8, 2017
@cezaraugusto
Copy link
Contributor

I referenced this issue in #9809 but that's wrong. #9809 solved #8429. I referenced the wrong issue by mistake. Re-opening

@cezaraugusto cezaraugusto reopened this Jul 8, 2017
@alexwykoff alexwykoff modified the milestones: 0.19.x (Beta Channel), 0.18.x (Release Channel) Jul 18, 2017
@cezaraugusto cezaraugusto modified the milestones: 0.18.x (Beta Channel), 0.19.x (Developer Channel) Jul 25, 2017
@bbondy bbondy modified the milestones: 0.19.x (Developer Channel), 0.18.x (Beta Channel) Jul 25, 2017
cezaraugusto added a commit that referenced this issue Jul 26, 2017
@LaurenWags
Copy link
Member

@cezaraugusto - noticing that the tabs from the first tab set don't have their favicon resized the way the tabs from the other tab sets do. Could you take a look?
screen shot 2018-01-04 at 11 41 34 am

@cezaraugusto
Copy link
Contributor

could you share your STR? I think I know what's happening

@LaurenWags
Copy link
Member

LaurenWags commented Jan 4, 2018

just tried to confirm my steps were the same as from the description, but now i'm seeing this :(
screen shot 2018-01-04 at 11 58 41 am

@cezaraugusto is this expected? (the tabs are too small to display a favicon?)

@LaurenWags
Copy link
Member

Reopening per #9779 (comment) - I no longer see a tab favicon when I set the tab count to 100.

@LaurenWags LaurenWags reopened this Jan 11, 2018
@cezaraugusto
Copy link
Contributor

cezaraugusto commented Jan 17, 2018

@LaurenWags depending on your window size a tab with no icons is expected. However, #9779 (comment) is not. I'm not able to reproduce following @srirambv STR:

jan-17-2018 17-05-51

I have a guess-fix but hard to know it's working without reproducing. Let me know if there's something I'm missing

@cezaraugusto
Copy link
Contributor

^ sorry for the bad GIF quality

@cezaraugusto
Copy link
Contributor

also, could you tell me which version rev are you on? I'm on de5970f

@LaurenWags
Copy link
Member

So to recap some slack convos - can no longer repro #9779 (comment) I only see #9779 (comment) which makes sense as discussed because at some point the tabs are too small to display a favicon.

However, (maybe this needs a new issue?) If I have a full screen window, and 100 tabs, no favicons. But, if I restart Brave the favicons do display:
9779-blanktabsuntilrestart

This is using:
Brave | 0.20.19
V8 | 6.4.388.29
rev | 3d05e91
Muon | 4.7.3
OS Release | 16.7.0
Update Channel | Beta
OS Architecture | x64
OS Platform | macOS
Node.js | 7.9.0
Brave Sync | v1.4.2
libchromiumcontent | 64.0.3282.99

@cezaraugusto
Copy link
Contributor

@LaurenWags what happens in that state you mentioned in #9779 (comment) if you keep changing the number of tabs per tab set? does at that window size the icons are shown or just keep the same?

per your screenshot, you reached a point where icons should be visible, but if you have resized the window before by increasing the width, it might not be updated at the right time given the number of tabs.

The time for icons to update based on tab size is variable and depends on how fast you resize, for slow resizing, components take more time to update given the operation cost in CPU. Fast resizes usually update instantly. An app restart as you did force the update which is why you see the icons.

If changing tab set size with the window at the same width do update the icons IMO we can close this. The delay of updating icons on window resize is variable and while not really a bug, could be improved over time and best take would be opening another issue.

@LaurenWags
Copy link
Member

Thanks @cezaraugusto and will open up something separate 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.