Closed Bug 1437433 Opened 4 years ago Closed 4 years ago

Add automated test for "Websites can be bookmarked from a private window"


(Firefox :: Bookmarks & History, enhancement, P2)




Firefox 60
Tracking Status
firefox60 --- fixed


(Reporter: pauly, Assigned: pauly)


(Blocks 1 open bug)


(Whiteboard: [fxsearch])


(1 file, 1 obsolete file)

TestRail link:

1. Launch Firefox.
2. Open a private window from menu [≡] → New Private Window.
3. Open a website that's currently not in your bookmarks list.
4. Click the star-shaped button to bookmark it.

Expected results:
1. Firefox is successfully launched.
2. A new private window is successfully opened.
3. The website in question is successfully loaded.
4. On click, the star-shaped button shows an animation and changes its color to blue.
The website is successfully bookmarked and no errors are thrown for this action.
Attached patch browser_bookmark_private_window (obsolete) — Splinter Review
Assignee: nobody → paul.silaghi
Attachment #8950141 - Flags: review?(standard8)
Comment on attachment 8950141 [details] [diff] [review]

Review of attachment 8950141 [details] [diff] [review]:

Looks good, just a couple of comments/queries to address.

::: browser/components/places/tests/browser/browser_bookmark_private_window.js
@@ +16,5 @@
> +
> +add_task(async function test_add_bookmark_from_private_window() {
> +  let win = await BrowserTestUtils.openNewBrowserWindow({private: true});
> +  let tab = win.gBrowser.selectedTab = win.gBrowser.addTab(TEST_URL);
> +  await BrowserTestUtils.browserLoaded(tab.linkedBrowser);

We can simplify these two lines to:

let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);

`openNewForegroundTab` automatically waits for the load to complete.

@@ +34,5 @@
> +  Assert.equal(bookmarkStar.getAttribute("starred"), "true", "Bookmark star changed its state correctly.");
> +
> +  // Check if the "Page Bookmarked" panel is open.
> +  let bookmarkPanelTitle = win.document.getElementById("editBookmarkPanelTitle");
> +  Assert.equal(bookmarkPanelTitle.value, "Page Bookmarked", "Bookmark panel title is correct.");

I think this is already checked by the fact that we've done the `await shownPromise`, isn't it?
Attachment #8950141 - Flags: review?(standard8)
Thanks for the review.
Added the requested changes.
Attachment #8950141 - Attachment is obsolete: true
Attachment #8951573 - Flags: review?(standard8)
Comment on attachment 8951573 [details] [diff] [review]

Great, thanks.
Attachment #8951573 - Flags: review?(standard8) → review+
Priority: -- → P2
Whiteboard: [fxsearch]
Pushed by
Add a test for checking that a website can be bookmarked from a private window. r=standard8
Keywords: checkin-needed
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in before you can comment on or make changes to this bug.