1429364 - The "Save Link as" context menu option do not work in the "Firefox Notes" sidebar

Status: VERIFIED FIXED

(Firefox :: General, defect, P2)

Description

[Marius Coman [:mcoman], Ecosystem QA]

Attachment: rec of the issue.gif

[Affected versions]:
- Firefox 57.0.4 and up

[Affected Platforms]:
- All Windows
- All Mac
- All Linux

[Prerequisites]:
- Have a Firefox profile with the latest version of the "Firefox Notes" add-on (2.2.0dev custom build from 01.10.2018) installed.
- Have a website link copied to clipboard.

[Steps to reproduce]:
1. Open the browser with the profiles from prerequisites.
2. Paste the link from prerequisites into the "Notes" sidebar.
3. Select the link and then right click on it.
4. Click the "Save Link as" context menu option and observe the behavior.

[Expected result]:
- The OS's specific dialog box is opened.

[Actual result]:
- Nothing happens.

[Notes]:
- Attached a screen recording of the issue.

Comment 1

[Andy McKay]

This doesn't work for a normal HTML page opened in the sidebar either. It's not a WebExtensions specific issue, moving over to Firefox.

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Gijs, do you know if we have a bug on file for this already?

Comment 3

[:Gijs (he/him)]

(In reply to Panos Astithas [:past] (please ni?) from comment #2)
> Gijs, do you know if we have a bug on file for this already?

bug 1285012 is the closest, though it doesn't mention this particular item. The context menu in the sidebar has long been iffy for many things. I don't know who would be a good person to go through and audit it in all its forms.

Bouncing this for you to be able to prio / assign someone / dupe / organize / etc. :-)

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

Nice find! Based on Mike's work in that bug I have a pretty simple fix that seems to do the trick. Let's see what try thinks:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2cc60d4c6dd067bfd1e7e9b8a46d573061966809

Comment 5

[:Gijs (he/him)]

(In reply to Panos Astithas [:past] (please ni?) from comment #4)
> Nice find! Based on Mike's work in that bug I have a pretty simple fix that
> seems to do the trick. Let's see what try thinks:
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=2cc60d4c6dd067bfd1e7e9b8a46d573061966809

Lovely. Pre-emptive r=me on that patch (also, nice all-green try!).

Can we have a follow-up to add some tests for context-menu-in-sidebar stuff? I bet it's possible to adapt the current set of tests that we have for the content context menu so that they run the exact same tests against the sidebar as well, which might unearth more issues (though I'm not sure how many of the context menus we actually activate, I think we mostly check that their visibility/enabled state is updated correctly - that'd still be a win over the current state, though!)

Also, from auditing based on that change, looks like view partial source is also broken:

https://dxr.mozilla.org/mozilla-central/rev/c38d22170d6f27e94c3c53093215d20255fab27a/browser/base/content/nsContextMenu.js#884

I'll file a follow-up myself for that one.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Bug 1429364 - Fix 'Save Link As' context menu option in sidebars.

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

Comment 7

[:Gijs (he/him)]

Comment on attachment 8942252 [details]
Bug 1429364 - Fix 'Save Link As' context menu option in sidebars.

https://reviewboard.mozilla.org/r/212532/#review218246

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/392278bcc755
Fix 'Save Link As' context menu option in sidebars. r=Gijs

Comment 9

[Andy McKay]

Woot! There's a lot more sidebar users these days thanks to WebExtensions.

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/392278bcc755

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to :Gijs from comment #5)
> Can we have a follow-up to add some tests for context-menu-in-sidebar stuff?

Filed bug 1430281.

Comment 12

[Marius Coman [:mcoman], Ecosystem QA]

This behavior is no longer reproducible, now the "Save Link as" context menu option works in the "Firefox Notes" sidebar
Verified as fixed using latest Nightly 59.0a1 Build ID 20180114220708, on, Windows 10 x64, Ubuntu 14.04 x64 and Mac 10.12.

Comment 13

[Julien Cristau [:jcristau]]

Seems late for 58 (not a new bug, and we're at RC week), so I'll call this wontfix.