Closed Bug 1227662 Opened 4 years ago Closed 4 years ago

Video background is white instead of textured gray

Categories

(Firefox for Android :: Theme and Visual Design, defect)

45 Branch
All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- fixed
fennec 45+ ---

People

(Reporter: kats, Assigned: mfinkle)

Details

Attachments

(1 file)

In nightly, load http://video.webmfiles.org/big-buck-bunny_trailer.webm for example, the background used to be a textured gray color, now it's white. I suspect this was also broken by the themes change in bug 1223526.
tracking-fennec: --- → ?
Assignee: nobody → mark.finkle
tracking-fennec: ? → 45+
Have you had time to look into this?

I was going to say maybe this doesn't matter on mobile, since we don't have as much screen space to be blindingly white, but when I tried this out, it is definitely worse than it used to be.

Maybe we just forgot to include whatever resource is used for that background?
Flags: needinfo?(mark.finkle)
This patch adds the four files needed to add backgrounds to video and image documents. Tested locally.
Flags: needinfo?(mark.finkle)
Attachment #8715023 - Flags: review?(margaret.leibovic)
Comment on attachment 8715023 [details] [diff] [review]
fix-videoimage-docs v0.1

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

::: toolkit/themes/mobile/jar.mn
@@ +38,5 @@
>     skin/classic/global/media/error.png                     (global/media/error.png)
>     skin/classic/global/media/throbber.png                  (global/media/throbber.png)
>     skin/classic/global/media/videoClickToPlayButton.svg    (global/media/videoClickToPlayButton.svg)
> +   skin/classic/global/media/TopLevelImageDocument.css     (global/media/TopLevelImageDocument.css)
> +   skin/classic/global/media/TopLevelVideoDocument.css     (global/media/TopLevelVideoDocument.css)

Why did you need to add CSS files for these? Why couldn't you use the toolkit ones, similar to how we use the toolkit images.
(In reply to :Margaret Leibovic from comment #3)

> Why did you need to add CSS files for these? Why couldn't you use the
> toolkit ones, similar to how we use the toolkit images.

Because bug 1223526 removed all of the toolkit theme files that Mobile was not specifically using. This patch adds these specific files back. Fennec now uses a toolkit/themes/mobile based "theme" which has almost zero files in it.
(In reply to Mark Finkle (:mfinkle) from comment #4)
> (In reply to :Margaret Leibovic from comment #3)
> 
> > Why did you need to add CSS files for these? Why couldn't you use the
> > toolkit ones, similar to how we use the toolkit images.
> 
> Because bug 1223526 removed all of the toolkit theme files that Mobile was
> not specifically using. This patch adds these specific files back. Fennec
> now uses a toolkit/themes/mobile based "theme" which has almost zero files
> in it.

Oh, sorry, I was confused looking at the paths - I thought you were adding these files in /mobile, not /toolkit/themes/mobile.
Attachment #8715023 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/df69b3262b51
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment on attachment 8715023 [details] [diff] [review]
fix-videoimage-docs v0.1

Approval Request Comment
[Feature/regressing bug #]: bug 1223526
[User impact if declined]: visual color difference. doesn't really break functionality
[Describe test coverage new/current, TreeHerder]: visual verification
[Risks and why]: low risk theme fixup
[String/UUID change made/needed]: none
Attachment #8715023 - Flags: approval-mozilla-beta?
Attachment #8715023 - Flags: approval-mozilla-aurora?
Comment on attachment 8715023 [details] [diff] [review]
fix-videoimage-docs v0.1

recent regression, taking it. Should be in 45 beta 5.
Attachment #8715023 - Flags: approval-mozilla-beta?
Attachment #8715023 - Flags: approval-mozilla-beta+
Attachment #8715023 - Flags: approval-mozilla-aurora?
Attachment #8715023 - Flags: approval-mozilla-aurora+
beta 4 I meant
You need to log in before you can comment on or make changes to this bug.