Closed
Bug 1404999
Opened 7 years ago
Closed 7 years ago
Cannot click "properties" of a bookmark in bookmark-toolbar while bookmark website is loading.
Categories
(Firefox :: Bookmarks & History, defect)
Tracking
()
RESOLVED
FIXED
Firefox 58
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox56 | --- | wontfix |
firefox57 | --- | wontfix |
firefox58 | --- | fixed |
People
(Reporter: e412byoy7, Assigned: enndeakin)
References
Details
(Keywords: regression)
Attachments
(1 file)
3.57 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0 Build ID: 20170926190823 Steps to reproduce: Click bookmark left click, then instantly right click, "properties" will be grey so not able to click it. Actual results: properties is grey Expected results: properties window should be able to be opened while the website of the bookmark is loading.
Summary: Cannot click "properties" of a bookmark while bookmark website is loading. → Cannot click "properties" of a bookmark in bookmark-toolbar while bookmark website is loading.
WFM in Fx56 x86 on Win10.
Component: Untriaged → Bookmarks & History
I have 64 bit Win 10 creators update but not fall creators update. you?
Flags: needinfo?(yfdyh000)
And you have Firefox on SSD? I have it installed on HDD which needs a few ms to read/write... But haven't had this issue in FF 54 and iirc neither in 55...
Win10 1703 on HDD. Try https://mozilla.github.io/mozregression/.
Flags: needinfo?(yfdyh000)
Keywords: regression,
regressionwindow-wanted
Comment 5•7 years ago
|
||
I think this could be related to bug 1395619 - which is an intermittent failure we've been seeing. I can't reproduce directly on my Mac, but I do seem to be able to get this to reproduce 100%: 1) Have some pinned tabs & normal tabs open. Have some bookmarks in the bookmarks toolbar. 2) Select one of the pinned tabs (so that selecting a bookmark on the toolbar will open in new tab). 3) Click a bookmark, and immediately right-click Actual results => The context menu is entirely disabled Expected results => The context menu is enabled
Depends on: 1395619
Comment 6•7 years ago
|
||
Hi, Daniel Can you please tell me if the issue is reproducible, using Latest Nightly 58.0a1?
Flags: needinfo?(danielboontje)
Comment 7•7 years ago
|
||
For my steps, mozregression says: 36:05.23 INFO: Last good revision: 9be2eb8c9061cdfc16d7d53e8f384afa5e1dcb8e 36:05.23 INFO: First bad revision: 56b70ce1a2ab369dd11b86d13d2966f8173ca356 36:05.23 INFO: Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9be2eb8c9061cdfc16d7d53e8f384afa5e1dcb8e&tochange=56b70ce1a2ab369dd11b86d13d2966f8173ca356 I suspect bug 1354564, but I'm trying to confirm that. I would suspect that this is directly related to Daniel's issue - it just depends on timing.
Updated•7 years ago
|
Comment 8•7 years ago
|
||
I confirmed by local backout that the steps in comment 5 are caused by bug 1354564. Given the nature of that bug, I think it could cause the main issue here, and cause the race that we're seeing in bug 1395619.
Assignee | ||
Comment 9•7 years ago
|
||
This is the simplest fix.
Assignee: nobody → enndeakin
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(enndeakin)
Reporter | ||
Comment 10•7 years ago
|
||
Can this be fixed for FF Beta 57.0b6 this friday? :)
Flags: needinfo?(danielboontje)
Comment 11•7 years ago
|
||
I've just tested it locally and the patch seems to work. I also ran it through the unit tests about 10 times, and it didn't fail (I'd normally have expected bug 1395619 to be showing up within a few runs as well).
Reporter | ||
Comment 12•7 years ago
|
||
When is the earliest date (this friday?) for this patch/fix to be included in FF Beta?
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8914734 [details] [diff] [review] Force command updating to be enabled when a menu is opened. OK, don't see any issues with this. I don't see any reason to rush this into 57; the issue has been present for several versions.
Flags: needinfo?(enndeakin)
Attachment #8914734 -
Flags: review?(mconley)
Comment 14•7 years ago
|
||
Daniel, maybe next week :) Too late for a fix in Firefox 56 though.
Reporter | ||
Comment 15•7 years ago
|
||
oh ok! :) Next week, great!
Comment 16•7 years ago
|
||
Comment on attachment 8914734 [details] [diff] [review] Force command updating to be enabled when a menu is opened. Review of attachment 8914734 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, Enn! ::: browser/components/places/tests/browser/browser_click_bookmarks_on_toolbar.js @@ +133,5 @@ > +// enabled state of the menuitems is updated properly. > +add_task(async function quickContextMenu() { > + await SpecialPowers.pushPrefEnv({set: [ > + [PREF_LOAD_BOOKMARKS_IN_TABS, true] > + ]}) Nit: missing semi-colon, which I think is going to burn you with the latest ESLint stuff. @@ +150,5 @@ > + type: "contextmenu" > + }); > + await promise; > + > + Assert.notEqual(document.getElementById("placesContext_open").disabled, true, Alternatively: Assert.ok(!document.getElementById("placesContext_open").disabled, "Commands in context menu are enabled");
Attachment #8914734 -
Flags: review?(mconley) → review+
Comment 17•7 years ago
|
||
Pushed by neil@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/04daa6d238c7 force command updating to be enabled when a menu is opened, r=mconley
Reporter | ||
Comment 18•7 years ago
|
||
awesooome! :) Thanks so much! Can't wait for it to go into FF Beta soon!
Comment 19•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/04daa6d238c7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment 20•7 years ago
|
||
Will ride the train with 58
Reporter | ||
Comment 21•7 years ago
|
||
Thanks so much! Can we get this into FF 57 Beta 10 this friday pleeeease Sebastian? :)
Reporter | ||
Comment 22•7 years ago
|
||
cannot wait another 31 days for this to be fixed... -.-
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
Comment 23•7 years ago
|
||
Daniel, I realising this is affecting you quite a bit, but unfortunately we've already deemed this not critical enough to fix for 57. It will be fixed in the next versions that follow soon.
Flags: needinfo?(mconley)
Reporter | ||
Comment 24•7 years ago
|
||
Ah alright! :) Can I expect it in the very first FF 58 beta?
Flags: needinfo?(standard8)
Reporter | ||
Comment 26•7 years ago
|
||
When can we expect this in FF Beta? :/ Can't wait! :P
You need to log in
before you can comment on or make changes to this bug.
Description
•