1100842 - [Flame][Video]The background color of confirm page is transparent when you delete a video.

Status: VERIFIED FIXED

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

Description

[Sue]

[1.Description]:
[Flame v2.1&v2.2]
When you delete a video via Video app, the background color of confirm page is transparent.
See attachment(1402.mp4 and logcat_1402.txt)
Found time:14:02

[2.Testing Steps]: 
1. Launch Video app.
2. Select a video to play.
3. Tap the menu -> Delete.

[3.Expected Result]: 
3. The confirm page will pop up with gray background color.

[4.Actual Result]: 
3. The background color of confirm page is transparent, but the "Cancel" and "Delete" button display with gray background color.

[5.Reproduction build]: 
Flame 2.1 versions:
Gaia-Rev        81160ad79e5b4c21967418dd63f1a1d08d77924e
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/3572aa3e6766
Build-ID        20141117001201
Version         34.0

Flame 2.2 versions:
Gaia-Rev        ae3a84acaab80a5b35d5542d63e68462273c8a1b
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/2d0a51ef828d
Build-ID        20141117160200
Version         36.0a1

[6.Reproduction Frequency]: 
Always Recurrence,5/5

Comment 1

[Sue]

Attachment: video

Comment 2

[Sue]

Attachment: logcat

Comment 3

[Joshua Mitchell (Inactive)]

I'm not able to see this video - adding QA-Wanted to get a video of this issue.

Comment 4

[Duane Dixon [:ddixon]]

Attaching a video link and a screenshot of the issue.  I reproduced this issue in the Gallery app when attempting to delete multiple videos. 

Actual Results: Delete confirmation screen is completely transparent. 

Video URL: 
http://youtu.be/OcaxkINIEZE

Added screenshot (above). 

Device: Flame 2.2 (shallow flash, eng. build, 319 MB memory)
BuildID: 20141202025557
Gaia: 725685831f5336cf007e36d9a812aad689604695
Gecko: 22ad8a162cf3
Version: 37.0a1 (2.2)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 5

[Duane Dixon [:ddixon]]

Attachment: delete_overlay.png

Comment 6

[Duane Dixon [:ddixon]]

Branch Check 

Issue DOES NOT occur in Flame 2.0 (shallow flash, eng. build, 319 MB memory).

Actual Results: Delete overlay screen is gray and opaque. 

Device: Flame 2.0
BuildID: 20141202035701
Gaia: 8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko: 29222e215db8
Version: 32.0 (2.0)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32

Comment 7

[Joshua Mitchell (Inactive)]

[Blocking Requested - why for this release]: Regression, poor UX, and looks bad.

Requesting a regression-window

Comment 8

[Duane Dixon [:ddixon]]

B2G Inbound Regression Window

Last Working

Device: Flame 2.1
BuildID: 20140730112305
Gaia: b67ddd7d40b52e65199478b8d6631c2c28fdf41d
Gecko: b3cbce8a2b87
Version: 34.0a1 (2.1)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First Broken

Device: Flame 2.1
BuildID: 20140730122704
Gaia: 7198320c257340009a6d6d0e12e609058881d230
Gecko: 2e4c92e81490
Version: 34.0a1 (2.1) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0


Last Working Gaia and First Broken Gecko
Issue DOES NOT occur here. 
Gaia: b67ddd7d40b52e65199478b8d6631c2c28fdf41d
Gecko: 2e4c92e81490

Last Working Gecko and First Broken Gaia
Issue DOES occur here. 
Gaia: 7198320c257340009a6d6d0e12e609058881d230
Gecko: b3cbce8a2b87

Gaia Pushlog: 
https://github.com/mozilla-b2g/gaia/compare/b67ddd7d40b52e65199478b8d6631c2c28fdf41d...7198320c257340009a6d6d0e12e609058881d230

Possible Cause: 

 bug 1039631 - Remove gradients and patterns from shared components an…

Comment 9

[Joshua Mitchell (Inactive)]

From the 4 issues in the pushlog  bug 1039631 is the most suspect for causing this bug because it is setting background colors. Can you take a look Aus?

Comment 10

[Aus Lacroix [:aus]]

Likely related to 1039631. Will have a look this week. Should be a fairly simple fix. The dialog probably needs a static background color like the other app dialogs. I am guessing the Video app uses it's own dialog and that's why the changes to 1039631 broke it's dialog because we removed the images.

Comment 11

[Aus Lacroix [:aus]]

Yep. It was exactly as I thought. Not sure why the Video application doesn't use standard app dialogs. It would've avoided this but, the fix was easy enough. :)

Comment 12

[Aus Lacroix [:aus]]

Attachment: Pull Request - Use solid color for background of dialog like we do in standard AppDialogs

Comment 13

[Aus Lacroix [:aus]]

Also, I would block on this. It looks pretty odd, and unintentional. The fix is really simple and completely safe.

Comment 14

[Aus Lacroix [:aus]]

Test run is green! Hooray!

Comment 15

[David Flanagan [:djf]]

Comment on attachment 8532632 [details] [review]
Pull Request - Use solid color for background of dialog like we do in standard AppDialogs

r- because I can't figure out how this patch will actually fix the reported bug.

The screenshot shows the delete confirmation dialog with a transparent background. That dialog uses the <form id='confirm-dialog'> element. But this patch modifies the CSS for the "overlay" and "in-use-overlay" elements. Seems completely unrelated.

It may be a good idea to remove the gradient and pattern backgrounds from the video app, but note that the music and gallery apps also use the same images for their overlay screens, so removing that should probably be a question for UX (It would be fine with me to use solid backgrounds in all three apps, but that is probably something for a separate bug.)

If this is in fact unrelated to your work in 1039631 feel free to reassign it to me or to :russn for a fix.

Comment 16

[Aus Lacroix [:aus]]

(In reply to David Flanagan [:djf] from comment #15)
> Comment on attachment 8532632 [details] [review]
> Pull Request - Use solid color for background of dialog like we do in
> standard AppDialogs
> 
> r- because I can't figure out how this patch will actually fix the reported
> bug.

Indeed, this doesn't fix the correct issue but a different dialog.

> It may be a good idea to remove the gradient and pattern backgrounds from
> the video app, but note that the music and gallery apps also use the same
> images for their overlay screens, so removing that should probably be a
> question for UX (It would be fine with me to use solid backgrounds in all
> three apps, but that is probably something for a separate bug.)

UX has already decided to remove these gradients and patterns a while back in bug 1039631 at a global level. Some apps were missed because we assumed they all used AppDialogs which they didn't.

> If this is in fact unrelated to your work in 1039631 feel free to reassign
> it to me or to :russn for a fix.

I'm happy to update the pull request to fix the other dialogs as well in the video app. I'll have another review request soon.

Comment 17

[Aus Lacroix [:aus]]

Comment on attachment 8532632 [details] [review]
Pull Request - Use solid color for background of dialog like we do in standard AppDialogs

Second round for the patch. Updated CSS for the confirm dialog as well. Still has removal of the use of 'pattern' and 'gradient' which saves about 1M of memory a piece because even though the rendering surface is freed when they are not displayed the uncompressed png data still remains.

Comment 18

[Russ Nicoletti [:russn]]

Comment on attachment 8532632 [details] [review]
Pull Request - Use solid color for background of dialog like we do in standard AppDialogs

Looks good. Thanks for the patch.

Comment 19

[Aus Lacroix [:aus]]

Fixed on master: https://github.com/mozilla-b2g/gaia/commit/1d9ae9cca415ad093beba9521c429350e1f2b14d

Also, despite our original thoughts on this issue, it is not a regression. The fix for this issue was specific to the dialog used in the Video App and not the System level dialogs that were changed by the patch for bug 1039631.

Comment 20

[Aus Lacroix [:aus]]

Comment on attachment 8532632 [details] [review]
Pull Request - Use solid color for background of dialog like we do in standard AppDialogs

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): New bug.
[User impact] if declined: Dialog transparency makes it difficult to read the message presented.
[Testing completed]: On Device (Flame v180 / Latest Gecko + Gaia)
[Risk to taking this patch] (and alternatives if risky): Extremely low.
[String changes made]: None.

Comment 21

[Sue]

Attachment: verify_video.MP4

This issue has been verified successfully on Flame v2.2
See attachment: verify_video.MP4
Reproducing rate: 0/5
Flame 2.2 versions:
Gaia-Rev        e2a3e606675c346b6e6f35351a458040be599b09
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/f14dcd1c8c0b
Build-ID        20141214040212
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141214.073552
FW-Date         Sun Dec 14 07:36:04 EST 2014
Bootloader      L1TC00011880

Comment 22

[Aus Lacroix [:aus]]

Commit (v2.1): https://github.com/mozilla-b2g/gaia/commit/79286eafe67707d1330966c1b1413b2d0de595d9

Fixed.

Comment 23

[Sue]

The issue is verified not happen in latest build of Flame 2.1
Flame 2.1 build:
Gaia-Rev        73be51f998031f06db0cd660c0e388fa621c9f4c
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/05dd053f1d90
Build-ID        20150104001209
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150104.034656
FW-Date         Sun Jan  4 03:47:06 EST 2015
Bootloader      L1TC000118D0