1029336 - Only disable system items in context menu for certified apps

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Browser Chrome, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Right now we disable the "default items" in context menus for all apps. Only items explicitly added by apps are shown.

This isn't really how context menus work elsewhere on the web. It would be good to align better with the web here.

Apparently a bunch of our built-in apps aren't really prepared to deal with context menus right now though, so we should probably keep them disabled for certified apps. But it'd be good to avoid certified apps coming to rely on the lack of context menus.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We might want to make this depend on bug 1029343 so that we retain the ability for pages to do their own context menus using <menu>, like they can today.

Comment 2

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/daleharvey/gaia/commit/3f737b8a2d75e8bf52ed2228abc117f9c45a29e0

Comment 3

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/daleharvey/gaia/commit/458c89f11bde2245dd85e7c547c20ac68b16baa9

Comment 4

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/daleharvey/gaia/commit/0c869a1e7be5f8b1b4ec3b7889299c3d064bbd1a

Apologies for the bugmail, noticed a few nits after settings review

Didnt know whether isCertified or the current |if| would be better, but this matches current similiar checks, the test is basic but yay integrations tests and I have been adding to it in follow up bugs

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8446710 [details]
https://github.com/daleharvey/gaia/commit/0c869a1e7be5f8b1b4ec3b7889299c3d064bbd1a

As you said, implement AppWindow.prototype.isCertified() makes more sense to me.
Please have test and ask for review again if you feel need.

Comment 6

[Dale Harvey (:daleharvey)]

This both had a new integration test, an existing unit test failed due to the changed behaviour and added isCertified + a unit test for that, so I think its good without a new review, thanks

landed in: https://github.com/mozilla-b2g/gaia/commit/163f259fc7c8a01e3265c050094767e3ad523b80

Comment 7

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

I had to revert this for a Gaia-unit test failure on TBPL: https://tbpl.mozilla.org/php/getParsedLog.php?id=42788708&tree=B2g-Inbound

https://github.com/mozilla-b2g/gaia/commit/90bb898e8401f5fb98edcf38293073c5c26ab7bd

Comment 8

[Dale Harvey (:daleharvey)]

it worked locally, looks like a sandbox failure, probably needs added to the mock, apologies for the hassle, will do a try run first this time

Comment 9

[Dale Harvey (:daleharvey)]

re pushed to try, https://tbpl.mozilla.org/?tree=Gaia-Try&rev=29cb5eff1c747fe3d86651131f4dd429ce596f35

Comment 10

[Dale Harvey (:daleharvey)]

above try is fully green, relanded

https://github.com/mozilla-b2g/gaia/commit/d377fc62cc97908d6d8574cc8de86843d4ce5fe9