Closed Bug 1079215 Opened 6 years ago Closed 6 years ago

Invalid warning when user taps in update available with disabled wifi connection

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
2.1 S8 (7Nov)

People

(Reporter: lolimartinezcr, Assigned: mancas)

References

Details

Attachments

(6 files, 1 obsolete file)

Flame
User
2.2
Gecko-9968a8f
Gaia-778ebac

Pre-requisites:
1. Wifi disabled.
2. "app.update.2g.enabled" = true
 "app.update.wifi-prioritized" = true
3. Update available.

STRs:
1. User drag and drop utility try.
2. Tap "1 update available"

Actual result:
User can see a warning "Updates are downloaded via data connection when Wi-Fi is not availabe. Additional data charges may apply." 

Expected result
This warning shouldn't appear.
Attached image 2014-10-07-13-34-11.png
Depends on: 1039501
No longer depends on: 1039501
Blocks: 1039501
No longer blocks: 1039501
Blocks: 1039501
Assignee: nobody → b.mcb
Attachment #8502328 - Flags: review?(timdream)
Attached image no--warning.png
Attachment #8502329 - Flags: ui-review?(jelee)
Comment on attachment 8502328 [details] [review]
Warning is not longer shown

I don't think this is the right fix (removing the warning outright).
Attachment #8502328 - Flags: review?(timdream)
We probably need more context on this bug. Is this a recent regression? What's the regressed bug?

According to the commit log the warning was added all the way back at bug 824575. Have we changed the behavior somewhere to cause this bug?
Hey Tim, according to bug1039501, this warning should not appear here. Instead, the warning will be displayed once the user is going to download the update.
Flags: needinfo?(timdream)
In that case please ask etienne for review then -- I am not sure if the removal is complete here. Thanks!
Flags: needinfo?(timdream)
Comment on attachment 8502329 [details]
no--warning.png

Hi Manuel, 
It looks ok from the screen but I will need to review the entire behavior on phone to be safe. Please provide me github link, thanks!
Attachment #8502329 - Flags: ui-review?(jelee)
Comment on attachment 8502328 [details] [review]
Warning is not longer shown

Hey Etienne, could you take a look at the PR and tell us if everything is ok?

Thanks!
Attachment #8502328 - Flags: review?(etienne)
(In reply to Jenny Lee from comment #8)
> Comment on attachment 8502329 [details]
> no--warning.png
> 
> Hi Manuel, 
> It looks ok from the screen but I will need to review the entire behavior on
> phone to be safe. Please provide me github link, thanks!

Hi Jenny, here you have the link to the PR:

https://github.com/mozilla-b2g/gaia/pull/24980

Thanks!
Flags: needinfo?(jelee)
Comment on attachment 8502328 [details] [review]
Warning is not longer shown

lgtm, sorry I missed it while reviewing bug 1039501.
Attachment #8502328 - Flags: review?(etienne) → review+
Hi Manuel,
Not sure why but see attached for what I actually see when tap on system update notification with no internet connection. I think the message shown is proper in this case, but the download button is missing (note that this dialog format is not defined in building blocks, but we can discuss that in separate bug in the future). I would like to have it back in gray out form.
Let me know if this is a bug or something else. Thanks!
Flags: needinfo?(jelee)
(In reply to Jenny Lee from comment #12)
> Created attachment 8504008 [details]
> no internet connection.png
> 
> Hi Manuel,
> Not sure why but see attached for what I actually see when tap on system
> update notification with no internet connection. I think the message shown
> is proper in this case, but the download button is missing (note that this
> dialog format is not defined in building blocks, but we can discuss that in
> separate bug in the future). I would like to have it back in gray out form.
> Let me know if this is a bug or something else. Thanks!

Hi Jenny, I'm not sure about the expected behaviour here since I've not implemented it and I don't know if we have specs for this part. Could you provide the specs needed to handle this situation?

I suggest to remove this notification and when user tap in download, a prompt with the message will be displayed to make it consistent with bug1039501
Flags: needinfo?(jelee)
(In reply to Manuel Casas Barrado [:mancas] from comment #13)
> (In reply to Jenny Lee from comment #12)
> > Created attachment 8504008 [details]
> > no internet connection.png
> > 
> > Hi Manuel,
> > Not sure why but see attached for what I actually see when tap on system
> > update notification with no internet connection. I think the message shown
> > is proper in this case, but the download button is missing (note that this
> > dialog format is not defined in building blocks, but we can discuss that in
> > separate bug in the future). I would like to have it back in gray out form.
> > Let me know if this is a bug or something else. Thanks!
> 
> Hi Jenny, I'm not sure about the expected behaviour here since I've not
> implemented it and I don't know if we have specs for this part. Could you
> provide the specs needed to handle this situation?
> 
Unfortunately I won't be able to update the spec until later in the 2.2 development cycle.

> I suggest to remove this notification and when user tap in download, a
> prompt with the message will be displayed to make it consistent with
> bug1039501

I agree. Please do so for now and I can look into how to improve the UX here in later release, thanks for your help :)
Flags: needinfo?(jelee)
Comment on attachment 8502328 [details] [review]
Warning is not longer shown

Etienne, the patch has been modified according to the previous comments. Please take a look, and check if I've forgotten to remove anything about the old warnings.

Thanks!
Attachment #8502328 - Flags: review+ → review?(etienne)
Attached image no-connection.png
Attachment #8507683 - Flags: ui-review?(jelee)
Attached image no-connection-warning.png (obsolete) —
Attachment #8507684 - Flags: ui-review?(jelee)
Attachment #8507683 - Flags: ui-review?(jelee) → ui-review+
Comment on attachment 8507684 [details]
no-connection-warning.png

Looks good to me but still ni Matej for string review.

Hi Matej,
This string is for when user tries to download system update without internet connection. please take a look, thanks!
Flags: needinfo?(Mnovak)
Attachment #8507684 - Flags: ui-review?(jelee)
Comment on attachment 8502328 [details] [review]
Warning is not longer shown

Thanks!
Attachment #8502328 - Flags: review?(etienne) → review+
(In reply to Jenny Lee from comment #18)
> Comment on attachment 8507684 [details]
> no-connection-warning.png
> 
> Looks good to me but still ni Matej for string review.
> 
> Hi Matej,
> This string is for when user tries to download system update without
> internet connection. please take a look, thanks!

Can we remove "Connection unavailable" and just keep the second line?
Flags: needinfo?(Mnovak)
Attachment #8507684 - Attachment is obsolete: true
Attachment #8508519 - Flags: ui-review?(Mnovak)
That looks good to me. Everyone else OK with that?
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/commit/f66b6e2f31a379cad208d28099d2f8fbaf042630
Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S8 (7Nov)
Tested and working
flame
user
master
188based
Gecko-aefcebe
Gaia-7256858
Status: RESOLVED → VERIFIED
Attachment #8508519 - Flags: ui-review?(matej) → ui-review+
You need to log in before you can comment on or make changes to this bug.