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

Background pages don't preserve bookmarked icon when later clicked. #8977

Closed
luixxiul opened this issue May 20, 2017 · 3 comments
Closed

Background pages don't preserve bookmarked icon when later clicked. #8977

luixxiul opened this issue May 20, 2017 · 3 comments

Comments

@luixxiul
Copy link
Contributor

luixxiul commented May 20, 2017

Test plan

#9003 (comment)


Describe the issue you encountered: Bookmarked page opened with middle mouse button does not have the filled star icon, until the page is completely loaded

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

  • Brave Version (revision SHA): 0.15.306

  • Steps to reproduce:

    1. Open https://github.com
    2. Bookmark the page
    3. Click the middle mouse button to open it in a new tab
  • Actual result: The tab does not have the filled star icon

  • Expected result: It should.

  • Is this an issue in the currently released version?
    No. The star icon is filled as soon as the page starts loading, even if it is not at the same time. The lag became noticeable on 0.15.306.

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:

  • Any related issues:

@luixxiul
Copy link
Contributor Author

clipboard01

@bbondy bbondy changed the title Bookmarked page opened with middle mouse button does not have the filled star icon Background pages don't preserve bookmarked icon when later clicked. May 22, 2017
@bbondy
Copy link
Member

bbondy commented May 22, 2017

I don't have a fix yet but I wrote a test for this:

diff --git a/test/bookmark-components/bookmarksTest.js b/test/bookmark-components/bookmarksTest.js
index 9d7e7098..7cf8f2c6 100644
--- a/test/bookmark-components/bookmarksTest.js
+++ b/test/bookmark-components/bookmarksTest.js
@@ -1,4 +1,4 @@
-/* global describe, it, before */
+/* global describe, it, before, beforeEach */
 
 const Brave = require('../lib/brave')
 const Immutable = require('immutable')
@@ -249,6 +249,41 @@ describe('bookmark tests', function () {
     })
   })
 
+  describe('bookmark star button is preserved', function () {
+    Brave.beforeEach(this)
+    beforeEach(function * () {
+      this.page1Url = Brave.server.url('page1.html')
+      this.page2Url = Brave.server.url('page2.html')
+      yield setup(this.app.client)
+      yield this.app.client
+        .addSite({
+          location: this.page1Url,
+          folderId: 1,
+          parentFolderId: 0,
+          tags: [siteTags.BOOKMARK]
+        }, siteTags.BOOKMARK)
+    })
+
+    it('on new active tabs', function * () {
+      yield this.app.client
+        .waitForVisible(navigatorNotBookmarked)
+        .newTab({ url: this.page1Url })
+        .waitForVisible(navigatorBookmarked)
+    })
+    it('on new active tabs', function * () {
+      yield this.app.client
+        .waitForVisible(navigatorNotBookmarked)
+        .newTab({ url: this.page1Url, active: false })
+        .waitForUrl(this.page1Url)
+        .tabByIndex(0)
+        .loadUrl(this.page2Url)
+        .waitForUrl(this.page2Url)
+        .windowByUrl(Brave.browserWindowUrl)
+        .ipcSend('shortcut-next-tab')
+        .waitForVisible(navigatorBookmarked)
+    })
+  })
+
   describe('menu behavior', function () {
     Brave.beforeAll(this)
 

@bsclifton bsclifton assigned bsclifton and ayumi and unassigned bsclifton May 22, 2017
ayumi added a commit that referenced this issue May 22, 2017
Fix #8977

Auditors: @bsclifton @bbondy

Test Plan:
1. Open https://github.com
2. Bookmark the page
3. Click the middle mouse button to open it in a new tab and switch to the new tab quickly (before it finishes loading)
4. Tab should have bookmark star icon
@ayumi ayumi mentioned this issue May 22, 2017
7 tasks
bsclifton pushed a commit that referenced this issue May 23, 2017
Fix #8977

Auditors: @bsclifton @bbondy

Test Plan:
1. Open https://github.com
2. Bookmark the page
3. Click the middle mouse button to open it in a new tab and switch to the new tab quickly (before it finishes loading)
4. Tab should have bookmark star icon
@luixxiul
Copy link
Contributor Author

the fix works like a charm, thanks.

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