Closed Bug 802946 Opened 7 years ago Closed 7 years ago

Cleanup the 'Hide Apps/Promo' mess

Categories

(Firefox for Android :: General, defect)

16 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 19
Tracking Status
firefox17 --- verified
firefox18 --- verified
firefox19 --- verified

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

(Whiteboard: [mess])

Attachments

(3 files, 1 obsolete file)

Bug 787188 landed some code I thought would be clever and migrate through the trains sparing us any grief. It did not, as show in bug 802653.

I landed a bandaid in bug 802653. This bug will clean it up in a better way.

Instead of using preprocessing magic and MOZ_UPDATE_CHANNEL, we'll simply backout the code we don't want on Beta.

I plan to add a few patches here:
1. Backout the bandaid from bug 802653 in Beta
2. Backout the original patch from all branches
3. Remove the App menu and promo code from Beta

Then I'll add a reference to #3 in this wiki so we can remember to keep backing it out for as long as needed:
https://wiki.mozilla.org/Release_Management/Merge_Documentation#Known_Backouts_for_Version
This patch only simply removes the bustage fix so I can more fully remove all the offending code

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 802653
User impact if declined: none - only maintenance issue
Testing completed (on m-c, etc.): beta only
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none
Assignee: nobody → mark.finkle
Attachment #672673 - Flags: review?(blassey.bugs)
Attachment #672673 - Flags: approval-mozilla-beta?
Straight backout of patch in bug 787188. It turned out to be not-so-clever

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 787188
User impact if declined: none - maintenance issue
Testing completed (on m-c, etc.): will need to backout on nightly, aurora and beta
Risk to taking this patch (and alternatives if risky): low, but we will need to followup with a patch to remove the App mennu and promo UI
String or UUID changes made by this patch:
Attachment #672675 - Flags: review?(blassey.bugs)
Attachment #672675 - Flags: approval-mozilla-beta?
Attachment #672675 - Flags: approval-mozilla-aurora?
This patch:
* removes the "Apps" menu from all menu XML
* removes the menu handler for the Apps menu
* removes the possibility that the Apps promo will ever be used
* falls back to use no promo if Sync is already set up

[Approval Request Comment]
Bug caused by (feature/regressing bug #): needed to really fix bug 787188
User impact if declined: marketing will not be happy
Testing completed (on m-c, etc.): this will only affect beta, and flow to release
Risk to taking this patch (and alternatives if risky): low, we have been doing the same thing with preprocessing for a while now.
String or UUID changes made by this patch: none
Attachment #672676 - Flags: review?(blassey.bugs)
Attachment #672676 - Flags: approval-mozilla-beta?
OS: Linux → Android
QA Contact: aaron.train
Hardware: x86_64 → ARM
Whiteboard: [mess]
Comment on attachment 672675 [details] [diff] [review]
patch 2 - backout the original preprocessor based patch

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

You're deleting AboutHomeContent.java.in and adding a new AboutHomeContent.java, which will kill off any blame information. Please use "hg mv" instead to keep the blame information.

Also, I commented in bug 787188 now that I've seen this, but I really don't like AboutHomeContent.java becoming preprocessed. Let's undo that (proposed solution in bug 787188).

r- simply because the add/remove makes it near impossible to see what changes are being made to this file
Attachment #672675 - Flags: review?(blassey.bugs) → review-
Attachment #672676 - Flags: review?(blassey.bugs) → review+
Comment on attachment 672673 [details] [diff] [review]
patch 1 - remove bandaid fix from Beta

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

r+, conditional on a new, reviewed backout patch
Attachment #672673 - Flags: review?(blassey.bugs) → review+
Triage drive by: waiting for patch 2 to get r+ and land on trunk before approving anything for beta uplift.
I manually | hg mv | this patch instead of using a backout script.
Attachment #672675 - Attachment is obsolete: true
Attachment #672675 - Flags: approval-mozilla-beta?
Attachment #672675 - Flags: approval-mozilla-aurora?
Attachment #673347 - Flags: review?(blassey.bugs)
(In reply to Brad Lassey [:blassey] from comment #4)

> Also, I commented in bug 787188 now that I've seen this, but I really don't
> like AboutHomeContent.java becoming preprocessed. Let's undo that (proposed
> solution in bug 787188).

I mentioned in comment 0 that I would be backing out the original patch from _all_ branches. There will be no preprocessing left when we are done.
Comment on attachment 673347 [details] [diff] [review]
patch 2 v2 - backout the original preprocessor based patch

[Approval Request Comment]
See previous request...
Attachment #673347 - Flags: approval-mozilla-beta?
Attachment #673347 - Flags: approval-mozilla-aurora?
Attachment #673347 - Flags: review?(blassey.bugs) → review+
Attachment #672673 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #672676 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #673347 - Flags: approval-mozilla-beta?
Attachment #673347 - Flags: approval-mozilla-beta+
Attachment #673347 - Flags: approval-mozilla-aurora?
Attachment #673347 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/618a14edc7e0
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.