Closed Bug 933428 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox for Android :: Awesomescreen, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED WONTFIX
Firefox 28
Tracking Status
firefox26 --- verified
firefox27 --- verified
firefox28 --- verified
b2g-v1.2 --- fixed

People

(Reporter: Margaret, Assigned: Margaret)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

Part of the design for bug 931021.
To reduce UI churn for users, it would be nice to include this fix in 26 when we ship the new about:home.
tracking-fennec: --- → ?
Assignee: nobody → margaret.leibovic
Attached patch patchSplinter Review
These strings are used in other context menu items, so we don't need to worry about removing them (handy for uplifting!).
Attachment #825516 - Flags: review?(sriram)
Comment on attachment 825516 [details] [diff] [review]
patch

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

Nice cleanup.
Attachment #825516 - Flags: review?(sriram) → review+
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
Attached patch test fixSplinter Review
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.
Attachment #826517 - Flags: review?(gbrown)
Attachment #826517 - Flags: review?(gbrown) → review+
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
Attachment #825516 - Flags: approval-mozilla-beta?
Attachment #825516 - Flags: approval-mozilla-aurora?
Comment on attachment 826517 [details] [diff] [review]
test fix

[Approval Request Comment]
test fix needed to prevent patch from breaking the tree
Attachment #826517 - Flags: approval-mozilla-beta?
Attachment #826517 - Flags: approval-mozilla-aurora?
I don't understand why this was removed but nonetheless the patches work on Nightly (11/05).
Status: RESOLVED → VERIFIED
(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.
Attachment #825516 - Flags: approval-mozilla-beta?
Attachment #825516 - Flags: approval-mozilla-beta+
Attachment #825516 - Flags: approval-mozilla-aurora?
Attachment #825516 - Flags: approval-mozilla-aurora+
Attachment #826517 - Flags: approval-mozilla-beta?
Attachment #826517 - Flags: approval-mozilla-beta+
Attachment #826517 - Flags: approval-mozilla-aurora?
Attachment #826517 - Flags: approval-mozilla-aurora+
Looks like this has dependencies that need uplifting first.
Flags: needinfo?(margaret.leibovic)
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.
tracking-fennec: ? → ---
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.
First, I want to backout of Beta since we are running out of time.
Attachment #8338513 - Flags: review?(wjohnston)
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
Attachment #8338513 - Flags: approval-mozilla-beta?
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Attachment #8338530 - Flags: review?(wjohnston)
Attachment #8338530 - Flags: approval-mozilla-aurora?
Attachment #8338530 - Flags: review?(wjohnston) → review+
Attachment #8338513 - Flags: review?(wjohnston) → review+
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.
Attachment #8338513 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8338530 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Shouldn't this also get backed out on trunk for complete removal until this UI change is revisited?
Flags: needinfo?(margaret.leibovic)
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
Attachment #8338826 - Flags: review?(wjohnston)
Attachment #8338826 - Flags: review?(wjohnston) → review+
Flags: needinfo?(margaret.leibovic)
So what's happening here? Close bug?
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.