1388753 - Bookmarks and history pages in Library sub menus are not working

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P1)

Description

[Edo Karabas]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170809100326

Steps to reproduce:

1. Click on Hamburger menu - Library or Library from Toolbar
2. Click on Bookmarks or History sub menu
3. Click on item from Recently added Bookmarks or Recent History



Actual results:

Nothing is happening.


Expected results:

It shoudl open a recently added bookmark or a page from recent history

Comment 2

[:Gijs (he/him)]

Attachment: Bug 1388753 - unbreak bookmarks/history item clicks and tidy up code in browserPlacesViews.js,

Review commit: https://reviewboard.mozilla.org/r/166650/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/166650/

Comment 3

[Mike de Boer [:mikedeboer]]

Comment on attachment 8895449 [details]
Bug 1388753 - unbreak bookmarks/history item clicks and tidy up code in browserPlacesViews.js,

https://reviewboard.mozilla.org/r/166650/#review172258

LGTM!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf603affd1e0
unbreak bookmarks/history item clicks and tidy up code in browserPlacesViews.js, r=mikedeboer

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing browser-chrome's browser_947914_button_history.js:

https://hg.mozilla.org/integration/autoland/rev/5475c7748903350a91842046eeb614880738b83e

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bf603affd1e09700f29576cfcae62d2ba07559ea&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=122311575&repo=autoland

08:04:06     INFO - TEST-PASS | browser/components/customizableui/test/browser_947914_button_history.js | Should have a history item for the history we just made. - 
08:04:06     INFO - Buffered messages finished
08:04:06     INFO - TEST-UNEXPECTED-FAIL | browser/components/customizableui/test/browser_947914_button_history.js | Test timed out - 
08:04:06     INFO - GECKO(3284) | 1502377446586	addons.xpi	WARN	Attempting to activate an already active default theme
08:04:06     INFO - Console message: 1502377446586	addons.xpi	WARN	Attempting to activate an already active default theme
08:04:06     INFO - Not taking screenshot here: see the one that was previously logged
08:04:06     INFO - TEST-UNEXPECTED-FAIL | browser/components/customizableui/test/browser_947914_button_history.js | A promise chain failed to handle a rejection: Panel did not hide within 20 seconds. - stack: JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: register :: line 199
08:04:06     INFO - JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: completePromise :: line 711
08:04:06     INFO - JS frame :: chrome://mochitests/content/browser/browser/components/customizableui/test/head.js :: promisePanelElementHidden/</timeoutId< :: line 315Rejection date: Thu Aug 10 2017 08:03:42 GMT-0700 (Pacific Standard Time) - false == true - JS frame :: resource://testing-common/PromiseTestUtils.jsm :: assertNoUncaughtRejections :: line 265
08:04:06     INFO - Stack trace:
08:04:06     INFO -     resource://testing-common/PromiseTestUtils.jsm:assertNoUncaughtRejections:265
08:04:06     INFO -     chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest<:448
08:04:06     INFO -     timeoutFn@chrome://mochikit/content/browser-test.js:893:9
08:04:06     INFO -     setTimeout handler*Tester_execTest@chrome://mochikit/content/browser-test.js:855:9
08:04:06     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:689:7
08:04:06     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59

Comment 7

[:Gijs (he/him)]

Comment on attachment 8895449 [details]
Bug 1388753 - unbreak bookmarks/history item clicks and tidy up code in browserPlacesViews.js,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166650/diff/1-2/

Comment 8

[:Gijs (he/him)]

I'd forgotten how awful trying to do any kind of automated tested with these panels was. Re-landed without the test because bug 1383009 will mean everyone will hit this in tomorrow's nightly otherwise.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/672d9c4258b3
unbreak bookmarks/history item clicks and tidy up code in browserPlacesViews.js, r=mikedeboer

Comment 10

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d0068f9051be
Backed out changeset bf603affd1e0 for failing browser-chrome's browser_947914_button_history.js. r=backout a=merge

Comment 11

[:Gijs (he/him)]

(In reply to Pulsebot from comment #10)
> Backout by [e-mail]:
> https://hg.mozilla.org/mozilla-central/rev/d0068f9051be
> Backed out changeset bf603affd1e0 for failing browser-chrome's
> browser_947914_button_history.js. r=backout a=merge

Why did you back this back out of autoland via a merge from m-c?

Comment 12

[:Gijs (he/him)]

Comment on attachment 8895449 [details]
Bug 1388753 - unbreak bookmarks/history item clicks and tidy up code in browserPlacesViews.js,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166650/diff/2-3/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd9ea3879cde
unbreak bookmarks/history item clicks and tidy up code in browserPlacesViews.js, r=mikedeboer

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/94efd7ef04b2
unbreak bookmarks/history item clicks and tidy up code in browserPlacesViews.js, r=mikedeboer

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/672d9c4258b3

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/94efd7ef04b2

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

As far as I can tell, comment 10 was the backout from comment 5, cherry-picked onto what I wanted to merge to m-c. Pulsebot saw that the revision had chamged due to the cherry picking, so decided to add a comment.

Comment 18

[:Gijs (he/him)]

(In reply to Wes Kocher (:KWierso) from comment #17)
> As far as I can tell, comment 10 was the backout from comment 5,
> cherry-picked onto what I wanted to merge to m-c. Pulsebot saw that the
> revision had chamged due to the cherry picking, so decided to add a comment.

This patch landed 3 times on autoland. The first one got backed out (comment 5). The second one (comment 9) got backed out by the merge from m-c to autoland in comment 10 (cf. https://hg.mozilla.org/integration/autoland/rev/fda2d8158679#l201.12 - warning: slow link because big merge commit), so I landed it a third time (comment 13) and Aryx then kindly landed it separately on m-c to ensure it made nightlies (comment 14). What I don't understand is why the merge in comment 10 reverted the second commit at all.

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I think mercurial just got confused with me cherry-picking the backout into the merge and then when that all merged back it blew away the second reland attempt.

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bd9ea3879cde

Comment 21

[u554753]

Verified on Windows, Mac, and Ubuntu.

Comment 22

[Bill his name]

https://bugzilla.mozilla.org/show_bug.cgi?id=1402384