Closed Bug 1545700 Opened 1 year ago Closed 1 year ago

Bookmarks Menu item is empty at first time popup

Categories

(Toolkit :: XUL Widgets, defect, P1)

68 Branch
x86_64
Windows 10
defect

Tracking

()

VERIFIED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox66 --- unaffected
firefox67 --- unaffected
firefox68 --- verified
firefox69 --- verified

People

(Reporter: alice0775, Assigned: bgrins)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

Reproducible: always

Steps To Reproduce:

  1. Start Nightly with new profile
  2. Right Click on title bar and Enable Menu bar
  3. Click on "Bookmarks" menu

If not reproduce,
4. Restart browser
5. Click on "Bookmarks" menu

Actual Results:
Some bookmarks Menu item are empty
See attached screenshot

Expected Results:
should not empty

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=27157cc6395ef681545b5a8ae5f95fc584f38792&tochange=223a8a0c4eed19547e6e843cfd265c728f7dc7c2

Suspect :
223a8a0c4eed Brian Grinstead — Bug 1519502 - Convert menu bindings to a Custom Element r=surkov

Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression
Summary: Bookmarks Menu item is empty at first time → Bookmarks Menu item is empty at first time popup

Bookmarks folder label are missing...

Assignee: nobody → alice0775
Assignee: alice0775 → bgrinstead
Priority: -- → P1

Neil, my best initial guess before I can test this on Windows is that the menus are being created after this code runs https://hg.mozilla.org/integration/autoland/rev/223a8a0c4eed#l13.238. If that's the case then I think isInMenuPopup should turn into isInClosedMenuPopup so we eagerly render if it's being inserted into an already-opened popup. I don't think there's currently an attribute set on menupopups like open that I could use to update that selector. I guess I could go ahead and add it right here in this function, but I'm guessing there's a better place. Do you think this could be done on the platform side when the popup is actually opened (and removed when it's closed)?

Flags: needinfo?(enndeakin)

Alice0775, I haven't been able to test this locally yet but I put together a patch that I think might fix it. I have a try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8be840e5002f879e228ddaa2dd29cb48a016714. Would you be able to test it please? It's an artifact build so the binaries should be ready in 10-20 minutes.

Flags: needinfo?(alice0775)

Not only bookmarks, context menu too.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1519502#c15

(In reply to Kazumasa Hasegawa (Kazz) from comment #5)

Not only bookmarks, context menu too.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1519502#c15

How can I reproduce that context menu issue? Is that entry coming from an addon?

How can I reproduce that context menu issue? Is that entry coming from an addon?

Yes.
That one is from my url2clipboard addon.

(In reply to Kazumasa Hasegawa (Kazz) from comment #7)

How can I reproduce that context menu issue? Is that entry coming from an addon?

Yes.
That one is from my url2clipboard addon.

Thanks for the info! I can confirm the context menu issue locally without my patch applied and that it's fixed with it applied.

I've pushed up a patch that fixes the addon context menu issue at https://bugzilla.mozilla.org/show_bug.cgi?id=1519502#c15 which I hope is the same fix needed for this bug. Would like confirmation from Alice0775 or Kazumasa if possible that it fixes the bookmarks menu issue, but if nothing else we could land the context menu fix and follow up if it doesn't.

(In reply to Brian Grinstead [:bgrins] from comment #4)

Alice0775, I haven't been able to test this locally yet but I put together a
patch that I think might fix it. I have a try push at
https://treeherder.mozilla.org/#/
jobs?repo=try&revision=d8be840e5002f879e228ddaa2dd29cb48a016714. Would you
be able to test it please? It's an artifact build so the binaries should be
ready in 10-20 minutes.

I cannot reproduce the problem on the try build(Windows 2012 x64 opt Ba). The try build fixed this problem.

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0
Built from https://hg.mozilla.org/try/rev/d8be840e5002f879e228ddaa2dd29cb48a016714

Flags: needinfo?(alice0775)
Duplicate of this bug: 1545776

Confirmed fixed on try too.

Thank you both for testing. Working to get the fix landed ASAP.

Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/895753bc084b
Eagerly render <menu> inserted into already-opened menupopup r=surkov
Duplicate of this bug: 1545881
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Flags: needinfo?(enndeakin)

I’ve tried to reproduce this issue following the str from Comment 0 on an affected build but without success.
Alice, could you please confirm this is fixed on latest beta?

Flags: needinfo?(alice0775)

(In reply to Maria Berlinger [:maria_berlinger], Release Desktop QA from comment #18)

I’ve tried to reproduce this issue following the str from Comment 0 on an
affected build but without success.
Alice, could you please confirm this is fixed on latest beta?

I can reproduce the issue on autoland 68.0a1 BuildID=20190419000651.
And I verified as fixed on Firefox68.0beta11 and Nightly69.0a1 BuildID=20190618041042.

Status: RESOLVED → VERIFIED
Flags: needinfo?(alice0775)

Thanks, Alice!
Removing the qe+ flag.

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.