If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

browser/base/content/test/general/browser_bookmark_popup.js permafails on cedar (presumably will perma-fails once 56 merges to beta)

RESOLVED FIXED in Firefox 56

Status

()

Firefox
Address Bar
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

55 Branch
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

https://treeherder.mozilla.org/#/jobs?repo=cedar&selectedJob=111728787

Several failures with messages like this:

07:49:30     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page is starred after closing - Got false, expected true
07:49:30     INFO - Stack trace:
07:49:30     INFO - chrome://mochikit/content/browser-test.js:test_is:967
07:49:30     INFO - chrome://mochitests/content/browser/browser/base/content/test/general/browser_bookmark_popup.js:test_bookmarks_popup/<:69
07:49:31     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page should only be starred prior to popupshown if editing bookmark - Got false, expected true
07:49:31     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page is starred - Got , expected true
07:49:31     INFO - Stack trace:
07:49:31     INFO - chrome://mochikit/content/browser-test.js:test_is:967
07:49:31     INFO - chrome://mochitests/content/browser/browser/base/content/test/general/browser_bookmark_popup.js:test_bookmarks_popup/<:52

I expect I made a mistake updating this test in bug 1352120.
Blocks: 1378197
(In reply to Marco Bonardo [::mak] from bug 1352120 comment #13)
> Comment on attachment 8880009 [details]
> Bug 1352120 - to fold: test changes,
> 
> https://reviewboard.mozilla.org/r/151346/#review156760
> 
> ::: browser/base/content/test/general/browser_bookmark_popup.js:14
> (Diff revision 2)
> >   */
> >  
> >  let bookmarkPanel = document.getElementById("editBookmarkPanel");
> > -let bookmarkStar = document.getElementById("bookmarks-menu-button");
> > +let bookmarkStar = AppConstants.MOZ_PHOTON_THEME ?
> > +  document.getElementById("star-button") :
> > +  document.getElementById("bookmarks-menu-button");
> 
> Maybe you could use the BookmarkingUI getters here (star or anchor?). The
> test seems to use this just for the starred attribute and a click

Of course, I should have checked this more carefully - BookmarkingUI.star and .anchor both return the inner elements for the pre-photon bookmarks-menu-button, and the 'starred' attribute is only set on the outer element, so now the test fails in non-photon.
Comment hidden (mozreview-request)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED

Comment 3

3 months ago
mozreview-review
Comment on attachment 8883397 [details]
Bug 1378161 - update browser_bookmark_popup to correctly deal with the non-photon case,

https://reviewboard.mozilla.org/r/154286/#review159532
Attachment #8883397 - Flags: review?(mak77) → review+

Comment 4

3 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d3964f81fbd4
update browser_bookmark_popup to correctly deal with the non-photon case, r=mak

Comment 5

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d3964f81fbd4
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Comment 6

3 months ago
27 failures in 656 pushes (0.041 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* cedar: 27

Platform breakdown:
* windows8-64: 10
* osx-10-10: 7
* windows7-32-vm: 5
* linux64: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1378161&startday=2017-07-03&endday=2017-07-09&tree=all
You need to log in before you can comment on or make changes to this bug.