792407 - Colour difference between title bar and in content UI background

Status: RESOLVED WORKSFORME

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

Description

[Ian Barlow (:ibarlow)]

Attachment: screenshot

The title bar background and html in-content UI backgrounds should match exactly, but there is a slight colour difference. 

We have confirmed that the background images being used are the same colour, so there appears to be some kind of difference in how we render colours in web content vs. UI.

Comment 1

[Sriram Ramasubramanian [:sriram]]

The media query tags added for reader mode should be added here too (with different textures).

Comment 2

[Chris Lord [:cwiiis]]

Working on this with a volunteer.

Comment 3

[Morrison Cole]

Attachment: WIP - Used the correct resource for page backgrounds.

Comment 4

[Morrison Cole]

Attachment: Used the correct resource for page backgrounds.

Used the correct background resource and toggle toolbar shadow on overscroll.

Comment 5

[Wesley Johnston (:wesj)]

Comment on attachment 679162 [details] [diff] [review]
Used the correct resource for page backgrounds.

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

Thanks for the patch. I don't think this is quite what we need here, but I think its an interesting idea. Can we split the shadow stuff off to a different bug and leave this one focused on the color issue?

::: mobile/android/base/gfx/GeckoLayerClient.java
@@ +695,5 @@
> +    private void setShadowVisibility() {
> +        String url = Tabs.getInstance().getSelectedTab().getURL();
> +        
> +        if (!(url == null) || !url.startsWith("about:")) {
> +            if (mPanZoomController.getOverscroll() == Axis.Overscroll.PLUS || mPanZoomController.getOverscroll() == Axis.Overscroll.NONE) {

Maybe i don't understand what you're trying to do, but I don't think this is what we want really. I think you're trying to show the shadow anytime the y-scroll position of the page isn't 0. This will only show if we're overscrolled in the positive direction or not overscrolled. We'll need to expose the scroll position somehow to do that.

I'd rather split this bit off to a different bug and make sure UX is ok with the change (show the urlbar shadow on about: pages when you scroll them. the transition is a little jarring to me) before we move forward with it, and leave this to try and figure out what's causing this color mismatch.

::: mobile/android/base/gfx/LayerView.java
@@ +322,5 @@
>          return BitmapFactory.decodeResource(getContext().getResources(), resId, options);
>      }
>  
>      Bitmap getBackgroundPattern() {
> +        return getDrawable(R.drawable.abouthome_bg);

I don't think this is the color that you want to change. This is what is shown "behind" web content in overscroll. You can see it on any web page if you pull down when the page is scrolled to its top. I don't think we want to change that color.

The issue here is that Gecko is for some reason rendering the images slightly different than Android. I would guess there's some sort of color profile problem:

https://developer.mozilla.org/en-US/docs/ICC_color_correction_in_Firefox

but these images don't have profiles that I can see. It may have to do with Android's resize algorythm?

Comment 6

[Wesley Johnston (:wesj)]

Ahh. I talked to Chris about this a bit. The idea here was to make the area behind web pages feel like it was an extension of the urlbar all the time. When you scroll about pages, we should show a shadow behind the page (not the urlbar) to reinforce that its "detached". I like that idea. Lets definitely file a new bug.

Comment 7

[Chris Lord [:cwiiis]]

The actual bug this patch addressed has been revised and refiled as bug 810278.

Comment 8

[Peter Retzer (:pretzer)]

I think the original issue still exists, therefore this should be a good candidate for the hackathon.

Comment 9

[Brian Nicholson (:bnicholson)]

I don't see this anymore; I think this was fixed when we change the colors to be 16-bit compatible.