933428 - Remove "Share" and "Add to Home Screen" from about:home context menus

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[:Margaret Leibovic]

Part of the design for bug 931021.

Comment 1

[:Margaret Leibovic]

To reduce UI churn for users, it would be nice to include this fix in 26 when we ship the new about:home.

Comment 2

[:Margaret Leibovic]

Attachment: patch

These strings are used in other context menu items, so we don't need to worry about removing them (handy for uplifting!).

Comment 3

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 825516 [details] [diff] [review]
patch

Review of attachment 825516 [details] [diff] [review]:
-----------------------------------------------------------------

Nice cleanup.

Comment 4

[:Margaret Leibovic]

I realized this is going to cause some tests to fail. Pushed a test update to try, let's see if it works:
https://tbpl.mozilla.org/?tree=Try&rev=9a7308651314

Comment 5

[:Margaret Leibovic]

Attachment: test fix

I only had to update testShareLink to deal with this change. I also updated the test to just extend BaseTest rather than AboutHomeTest, now that we don't have "Share" context menu items on about:home.

It looks like that permissions problem hack was added just for about:home swiping (bug 917398), so I decided to try getting rid of it. Things were green on try, so I think that's a safe move.

Comment 6

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/4e7de85bedc2
https://hg.mozilla.org/integration/fx-team/rev/ceb1e4a434a0

Comment 7

[:Margaret Leibovic]

Comment on attachment 825516 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new about:home
User impact if declined: We added these context menu items as part of the new about:home rewrite, but now we want to get rid of them. It would be nice to avoid UI churn if we never let them get to release.
Testing completed (on m-c, etc.): just landed on fx-team 
Risk to taking this patch (and alternatives if risky): low-risk, just removes two context menu items
String or IDL/UUID changes made by this patch: none

Comment 8

[:Margaret Leibovic]

Comment on attachment 826517 [details] [diff] [review]
test fix

[Approval Request Comment]
test fix needed to prevent patch from breaking the tree

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/4e7de85bedc2
https://hg.mozilla.org/mozilla-central/rev/ceb1e4a434a0

Comment 10

[Aaron Train [:aaronmt]]

I don't understand why this was removed but nonetheless the patches work on Nightly (11/05).

Comment 11

[:Margaret Leibovic]

(In reply to Aaron Train [:aaronmt] from comment #10)
> I don't understand why this was removed but nonetheless the patches work on
> Nightly (11/05).

See bug 931021 - we're trying to refine the context menus to make them less cluttered.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Looks like this has dependencies that need uplifting first.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/187487847908
https://hg.mozilla.org/releases/mozilla-aurora/rev/890695773e56

If bug 931843 gets approval for Fx26, this should be able to land with no extra help. Otherwise, it'll need some help for landing on beta.

Comment 14

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-beta/rev/d078b71b7015
https://hg.mozilla.org/releases/mozilla-beta/rev/0134540d3d48

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/d078b71b7015
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/0134540d3d48

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

We have more than a few people asking why we removed these menus. We also want these features, but we're thinking of a new UI to expose them. We also just made it a lot harder to "Add to Home Screen" from the application. It's only exposed in the URLbar context menu, which means you need the page loaded.

Bug 942463 means some people find the actions useful. Let's back this out ASAP and think a bit more about it.

Comment 17

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: Backout both patches (beta)

First, I want to backout of Beta since we are running out of time.

Comment 18

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8338513 [details] [diff] [review]
Backout both patches (beta)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): We removed a feature without a good replacement
User impact if declined: Complaints
Testing completed (on m-c, etc.): This is a backout
Risk to taking this patch (and alternatives if risky): low. the backout was simple.
String or IDL/UUID changes made by this patch: none

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: Backout both patches (aurora)

Comment 20

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8338513 [details] [diff] [review]
Backout both patches (beta)

As this returns us to a known good state, we can take this for final beta and get QA verification before ship.

Comment 21

[Lukas Blakk [:lsblakk] use ?needinfo]

Shouldn't this also get backed out on trunk for complete removal until this UI change is revisited?

Comment 22

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: Backout both patches (central)

I had some minor bitrot in the test files. The change that removed *.in file names and a test for the bookmark page context menu.

Try push looks good:
https://tbpl.mozilla.org/?tree=Try&rev=f8d5aa5cef8d

Comment 23

[Mark Finkle (:mfinkle) (use needinfo?)]

Backouts:

https://hg.mozilla.org/integration/fx-team/rev/7897e6dbbebf
https://hg.mozilla.org/releases/mozilla-aurora/rev/0e9f0f7a0f73
https://hg.mozilla.org/releases/mozilla-beta/rev/178a06da7cb3

Comment 24

[Aaron Train [:aaronmt]]

So what's happening here? Close bug?