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

Fixed invisible pinned tab after upgrade #8190

Closed
darkdh opened this issue Apr 10, 2017 · 2 comments
Closed

Fixed invisible pinned tab after upgrade #8190

darkdh opened this issue Apr 10, 2017 · 2 comments

Comments

@darkdh
Copy link
Member

darkdh commented Apr 10, 2017

Test plan

70ff720


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

  • Describe the issue you encountered:
    Pinned tabs

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

  • Brave Version (revision SHA):
    0.14.2 Preview 2

  • Steps to reproduce:

    1. Pin 4 tabs in 0.14.1
    2. Quit Brave
    3. Launch 0.14.2 preview 2
  • Actual result:
    We can only see one pined tab

  • Expected result:
    There should be 4 visible pinned tab

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

  • Is this an issue in the currently released version?

  • Can this issue be consistently reproduced?

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:

Kapture 2017-04-10 at 8.37.40.mp4.zip

  • Any related issues:
@darkdh darkdh added this to the 0.14.2 milestone Apr 10, 2017
@bsclifton
Copy link
Member

++ on this; I can re-add items back, but after quitting and re-launching Brave, all but one of the pinned tabs are removed

@bsclifton
Copy link
Member

bsclifton commented Apr 10, 2017

They site entries still have the appropriate tags (ex: "tags": ["pinned"],), but just aren't being displayed. They're not being displayed because pinnedLocation is undefined (not present in the JSON).

The pinnedLocation entries are being wiped out during the call to cleanAppData

@bsclifton bsclifton assigned bsclifton and unassigned bbondy Apr 10, 2017
bsclifton added a commit that referenced this issue Apr 11, 2017
At time addFrame is called, location isn't known which causes the check (which is removed by this commit) to only allow one pinned tab through (since url is undefined). The rest of the pinned tabs were dropped. This check is not needed.
Realistically, our sites map already has unique keys and we only need to check if the pinned site is unique when a new one is added (check is in app/browser/tabs).

Fixes #8190

Auditors: @bbondy

Test Plan:
1. Pin multiple sites
2. Exit
3. Re-launch and verify sites are still pinned
bsclifton added a commit that referenced this issue Apr 11, 2017
At time addFrame is called, location isn't known which causes the check (which is removed by this commit) to only allow one pinned tab through (since url is undefined). The rest of the pinned tabs were dropped. This check is not needed.
Realistically, our sites map already has unique keys and we only need to check if the pinned site is unique when a new one is added (check is in app/browser/tabs).

Fixes #8190

Auditors: @bbondy

Test Plan:
1. Pin multiple sites
2. Exit
3. Re-launch and verify sites are still pinned
@alexwykoff alexwykoff changed the title Invisible pinned tab after upgrade Fixed invisible pinned tab after upgrade Apr 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.