Closed Bug 1276374 Opened 4 years ago Closed 4 years ago

Remove firstrun Content Notifications slide

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 49
Tracking Status
firefox48 --- unaffected
firefox49 --- verified
fennec 49+ ---

People

(Reporter: liuche, Assigned: liuche)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Options are:

- Remove the slide altogether
- Replace it with a different slide
Barbara, Anthony - please comment with what you decide. Merge is next week.
Flags: needinfo?(bbermes)
Flags: needinfo?(alam)
tracking-fennec: --- → 49+
Spoke to Barbara about this, I think we should just remove it for now. I'll let her confirm with her NI. :)
Flags: needinfo?(alam)
Yes, agreed and OK'd, let's remove.
Flags: needinfo?(bbermes)
Summary: Handle Content Notifications firstrun slide → Remove firstrun Content Notifications slide
Comment on attachment 8758470 [details]
MozReview Request: Bug 1276374 - Remove firstrun Content Notifications slide. r=ahunt

https://reviewboard.mozilla.org/r/56708/#review53392
Attachment #8758470 - Flags: review?(ahunt) → review+
Pushed by cliu@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/a05fae3adc54
Remove firstrun Content Notifications slide. r=ahunt
Assignee: nobody → liuche
Comment on attachment 8758470 [details]
MozReview Request: Bug 1276374 - Remove firstrun Content Notifications slide. r=ahunt

Approval Request Comment
[Feature/regressing bug #]: In bug 1275418 we decided to disable content notifications, so we need to remove the firstrun slide describing this feature.
[User impact if declined]: firstrun slide may be confusing because there is no feature
[Describe test coverage new/current, TreeHerder]: local testing
[Risks and why]: very low, removing a line of code from a configuration
[String/UUID change made/needed]: String removal - but that can me omitted (and added to lint-ignore list)
Attachment #8758470 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/a05fae3adc54
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Attached image Cj8kgihXEAAxVKz.jpg
Verified as fixed using:
Device: One A2001 (Android 5.1.1)
Build: Firefox for Android Aurora 48.0a2 (2016-06-01)
Comment on attachment 8758470 [details]
MozReview Request: Bug 1276374 - Remove firstrun Content Notifications slide. r=ahunt

Sure, let's disable it
Attachment #8758470 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
has problems to apply to aurora:

grafting 347631:a05fae3adc54 "Bug 1276374 - Remove firstrun Content Notifications slide. r=ahunt"
merging mobile/android/base/java/org/mozilla/gecko/firstrun/FirstrunPagerConfig.java
merging mobile/android/base/locales/en-US/android_strings.dtd
merging mobile/android/base/strings.xml.in
warning: conflicts while merging mobile/android/base/java/org/mozilla/gecko/firstrun/FirstrunPagerConfig.java! (edit, then use 'hg resolve --mark')
warning: conflicts while merging mobile/android/base/locales/en-US/android_strings.dtd! (edit, then use 'hg resolve --mark')
warning: conflicts while merging mobile/android/base/strings.xml.in! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(liuche)
Comment on attachment 8758470 [details]
MozReview Request: Bug 1276374 - Remove firstrun Content Notifications slide. r=ahunt

Actually, my mistake - we don't need to uplift this because it's tracking 49, which has only just merged to aurora.
Flags: needinfo?(liuche)
Verified as fixed in build 49.0a2 (2016-06-15);
Device: 
- Nexus 5 (Android 6.0.1);
- Asus ZenPad 8 (Android 5.0.2);
- Motorola Razr (Android 4.4.4).
And based on comment 9 I will set the flag as verified.
Sylvestre, why did you set this to status-firefox48:affected? I believe this is only an issue on 49, but I want to be sure!
Flags: needinfo?(sledru)
I updated the status flag because of the uplift request.
Flags: needinfo?(sledru)
My bad, comment 12 addresses my concern. Looks like this was just confusion from this landing right around the merge date. All is good!
You need to log in before you can comment on or make changes to this bug.