Closed Bug 1359704 Opened 3 years ago Closed 3 years ago

ContextMenu context PAGE also appears in TAB

Categories

(WebExtensions :: General, defect)

54 Branch
defect
Not set

Tracking

(firefox54 fixed, firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: eros_uk, Assigned: Tomislav)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170421122919

Steps to reproduce:

I noticed that setting contexts: ['page'] in contextMenus.create also causes the contextmenu to appear in Tab Contextmenu.

While this is not a problem, it seems semantically incorrect.

N.B. Only tested on FF54. I am not aware if it was present in FF53.
looking into
Assignee: nobody → tomica
Attachment #8862427 - Flags: review?(mixedpuppy)
Comment on attachment 8862427 [details]
Bug 1359704 - "page" context items should not appear in "tab" context

https://reviewboard.mozilla.org/r/134354/#review137370
Attachment #8862427 - Flags: review?(mixedpuppy) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/dd7a3a0608d7
"page" context items should not appear in "tab" context r=mixedpuppy
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dd7a3a0608d7
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Out of curiosity, how come the fixed wasn't applied to upcoming FF54?
By default, patches land in Nightly (which is currently 55), and ride the train to release, unless it's a security / high-impact breaking bug, in which case one could ask for approval to uplift directly to beta/release.

While this bug is unfortunate, it doesn't seem like it would break any extensions, so I didn't ask for approval.  
Feel free to disagree, and give your reasons why.
As mentioned, I was curious. 

I was under the impression that any yet-unreleased version is still under development and therefore fixes are applied until the day the version is released to public.

I am currently on 54.0b2 and there are already nearly 20 minor updates to FF54 since I downloaded that version.
http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-beta-win32-add-on-devel/

I also thought that security updates will be applied regardless of the release/beta/nightly procedure ie as security update with minor version increments. 

Firefox 54 is due to be released 2017-06-13, 1.5 months away.
Leaving bugs (even minor one like this) for a release 3.5 months away (FF55) seems rather strange.

As you have said, it will not break any extension, but why wait to correct an error? :)
Every change, no matter how minor, can be risky, and you have to stop changing things at one point in order to test/stabilize before release.  We don't actually "stop", but limit to high-impact / low risk changes, or some favorable combination of those factors.

This is rather low-impact, but also admittedly very low risk, and I'm not actually sure what's the policy in cases like this.  Shane, do you think we wanna ask for uplift here?
Flags: needinfo?(mixedpuppy)
I think this change is low risk, but not very high gain.  

However, given it's a very small patch that fixes broken behavior, and it's relatively early in beta, I'd be ok to go with the risk of uplift.  Otherwise its over 4 months before the fix hits release.
Flags: needinfo?(mixedpuppy)
Comment on attachment 8862427 [details]
Bug 1359704 - "page" context items should not appear in "tab" context

Approval Request Comment
>[Feature/Bug causing the regression]:
bug 1316020

>[User impact if declined]:
Extension context menu items appear in the wrong context.

>[Is this code covered by automated tests?]:
Yes

>[Has the fix been verified in Nightly?]:
Yes

>[Needs manual test from QE? If yes, steps to reproduce]: 
No

>[List of other uplifts needed for the feature/fix]:
None

>[Is the change risky?]:
No

>[Why is the change risky/not risky?]:
Very small change, good test coverage.

>[String changes made/needed]:
None
Attachment #8862427 - Flags: approval-mozilla-beta?
Duplicate of this bug: 1356256
Comment on attachment 8862427 [details]
Bug 1359704 - "page" context items should not appear in "tab" context

Fix an extension context menu item issue. Beta54+. Should be in 54 beta 4.
Attachment #8862427 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Tomislav Jovanovic :zombie from comment #12)
> >[Is this code covered by automated tests?]:
> Yes
> 
> >[Has the fix been verified in Nightly?]:
> Yes
> 
> >[Needs manual test from QE? If yes, steps to reproduce]: 
> No

Setting qe-verify- based on Tomislav's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.