Transparent images should be rendered against the default background color

RESOLVED WONTFIX

Status

()

Core
Layout
RESOLVED WONTFIX
7 years ago
5 years ago

People

(Reporter: pascalc, Assigned: glazou)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

7 years ago
This is a follow up to Bug 376997 which allows images to be rendered centered and against a gray background (nice feature!).

As mentionned in the bug by several people, this causes a minor regression when you display a dark image with alpha transparency, the image is almost not visible. Before the patch, images were rendered against the default background color, in 99% of cases white, which can also be defined by the end-user in Firefox color preferences.

I think we should keep this behaviour since it would benefit from the new layout (centered image, gray background around the image) but would not change a behaviour we have always had regarding transparent images.
(Reporter)

Updated

7 years ago
Depends on: 376997
(Reporter)

Comment 1

7 years ago
Created attachment 584067 [details] [diff] [review]
patch for this bug

Here is a patch fixing the issue, I am asking the review from Boris because he did the review on the original patch in bug 376997 but really, feel free to move the review to whoever is available to do that, that's a tiny tiny patch and I feel bad pinging busy people for that.
Assignee: nobody → pascalc
Attachment #584067 - Flags: review?(bzbarsky)
Created attachment 584109 [details]
Image that does not benefit from white background

(In reply to Pascal Chevrel:pascalc from comment #0)
> Before the patch, images were rendered against the default
> background color, in 99% of cases white, which can also be defined by the
> end-user in Firefox color preferences.

Is there any data showing that 99% of transparent images are non-white? Any color chosen will can have issues with contrast. If a user right-clicks on the image and chooses "View Image Info", we will show the image on a white background.

This patch does not provide a necessarily better solution than the state of viewing images pre-Bug 376997. For example, see the attached image.
Version: 12 Branch → Trunk
UI-Review should be required for this bug.
Just a quick note on how I kind-of-solved this problem in ImageTweak: hitting the P key the background color is switched from custom/darkgray to default/white. Darkgray works for me 99.99% of the times, in the few cases when it doesn't (it really happened just once in 3 years) you hit P and you're fine.
(Reporter)

Comment 5

7 years ago
(In reply to Jared Wein [:jwein and :jaws] (Vacation December 23-January 3) from comment #2)

> This patch does not provide a necessarily better solution than the state of
> viewing images pre-Bug 376997. For example, see the attached image.

Of course, since the purpose of this patch and this bug is to restablish the state of viewing transparent images pre-Bug 376997.
(In reply to Pascal Chevrel:pascalc from comment #5)
> > This patch does not provide a necessarily better solution than the state of
> > viewing images pre-Bug 376997. For example, see the attached image.
> 
> Of course, since the purpose of this patch and this bug is to restablish the
> state of viewing transparent images pre-Bug 376997.

I honestly don't see the point of this... I can see the point in improving the situation; definitely not in restoring the status quo.
Comment on attachment 584067 [details] [diff] [review]
patch for this bug

While the CSS technically lives in /layout, it's a front-end feature.

I'm open to ideas for improving the display of such images, but this isn't it.
Attachment #584067 - Flags: review?(bzbarsky) → review-

Comment 8

7 years ago
subtle checkerboard?

Comment 9

7 years ago
(In reply to James May [:fowl] from comment #8)
> subtle checkerboard?

While checkerboard is a good way to make half-transparent parts easier to recognize it doesn't look very nice.

I think to make it look nice and be suitable for all images a way to switch between a light and a dark background (and a checkerboard?) would be good.

But how?
A preference - not easy enough to switch on a per image basis.
A keyboard shortcut - nobody will know about it.
A button - where should it be displayed?
(In reply to James May [:fowl] from comment #8)
> subtle checkerboard?

This doesn't work very well e.g. for bright/highly transparent images (think about a mostly-transparent white watermark)
Duplicate of this bug: 718376

Comment 12

6 years ago
Just came across this bug because I couldn't read text in an image with an alpha background and the new dark grey colour.  While I'm happy with the colour/border around the image, it's important to me to be to be able to change which colour shows through the transparent parts of the image.  Previously I could do this by changing the default background colour.

What about making it so that if you click on the image it switches between the default background and the default foreground?  Those colours can be customised already and are usually opposite light/dark, so making it easy to toggle between those two I think would be useful.
Duplicate of this bug: 720906

Comment 14

6 years ago
Can we not do what Photoshop CS6 proposes to do and offer a few variants of grey available in a context menu?

Comment 15

6 years ago
Created attachment 593357 [details]
Image that does benefit from white background

Any color chosen will can have issues with contrast, but, counting previous Fx behavior, it's regression.
Duplicate of this bug: 713904

Comment 17

6 years ago
This minor regression is not enough to delay the shipping of this new feature. Viewing stand-alone images is already and edge case and we've made it better for the overwhelming majority of images on the Web. For Web developers, it's a simple matter of turning off the styling with any of the fabulous web developer tools available (including a simple strip styles bookmarklet.)

As for a longterm solution, a toggle in the corner of the page that switches between the lightbox display and the stock background color would probably suffice, though I certainly wouldn't prioritize that fix over most others given how rarely used the feature is.

Comment 18

6 years ago
I disagree with your statement that "viewing standalone images is an edge case." Viewing standalone images is extremely common. Popular websites like Reddit are filled with links to standalone images. One of the most-used features of Google Image Search is "view full sized image" - a direct link to the image. Moreover, I'm not convinced that the majority of images on the web are photos which benefit from this change.

Comment 19

6 years ago
(In reply to Marcel Dejean from comment #18)
> I disagree with your statement that "viewing standalone images is an edge
> case." Viewing standalone images is extremely common. Popular websites like
> Reddit are filled with links to standalone images. One of the most-used
> features of Google Image Search is "view full sized image" - a direct link
> to the image. Moreover, I'm not convinced that the majority of images on the
> web are photos which benefit from this change.

Marcel, thanks for your input. I appreciate that from your perspective, where Reddit is a popular website and images with alpha transparency are commonplace, you might see things this way. If I was thinking about the sites I visit and thinking about my personal esthetic, I'd probably see it somewhat differently too. By my role is to think about this from the point of view of nearly half a billion users. For that audience, this flaw is not a blocker to this new feature.

Comment 20

6 years ago
Where the complaints from those half a billion users that the old behaviour was broken?  I'm not aware of any complaints, but when this feature goes out a huge number of people are going to be inconvenienced.  Maybe not half a billion people, but I'm sure many more than are at present.

Maybe this feature should be delayed until it doesn't inconvenience anyone?  A number of workable solutions have been proposed here.  Why not wait until those are implemented so as not to cause problems for large numbers of your users?
(In reply to Marcel Dejean from comment #18)
> I disagree with your statement that "viewing standalone images is an edge
> case." Viewing standalone images is extremely common. Popular websites like
> Reddit are filled with links to standalone images. One of the most-used
> features of Google Image Search is "view full sized image" - a direct link
> to the image. Moreover, I'm not convinced that the majority of images on the
> web are photos which benefit from this change.

Agreed.  Which also spill over into Facebook in the form of links to static images.  I think we're talking about a lot more than just Reddit alone.
(Assignee)

Comment 22

6 years ago
(In reply to Asa Dotzler [:asa] from comment #17)

> As for a longterm solution, a toggle in the corner of the page that switches
> between the lightbox display and the stock background color would probably
> suffice, though I certainly wouldn't prioritize that fix over most others
> given how rarely used the feature is.

I suggest you gather metrics before saying "rarely". Look at anchors targeting
an image w/o a rel attribute to avoid toolkits like lightbox.

Comment 23

6 years ago
As a web developer, I view standalone images with alpha transparency constantly. Drastically changing the basic behavior of this fundamental tool will only make Firefox less attractive as a development tool.

As a Redditor, Facebook user and general web citizen, I view standalone images of all types all the time. From this perspective, changing the behavior for this use case feels like arbitrary change for the sake of change.

This is a bad idea. Not quite as bad as Firefox adopting -webkit- CSS prefixes, but still bad.
(Assignee)

Comment 24

6 years ago
Created attachment 597420 [details] [diff] [review]
proposed fix

I am proposing a trivial fix for this bug that adds the hidden pref
|browser.standalone_images.background_color|. Please don't focus on the
name, that can be easily changed. I'm attaching a patch I did not have the
time to test but builds ok and is a good compromise between the explained
need for a dark background and the need of developers for a clear background.
It should then be enough for all users. Time needed to write this patch:
2 minutes, build included.
(In reply to Daniel Glazman from comment #24)
> Created attachment 597420 [details] [diff] [review]
> proposed fix
> 
> I am proposing a trivial fix for this bug that adds the hidden pref
> |browser.standalone_images.background_color|.

Thanks for the patch. I think we might need some security-check/parsing of the CSS color, similar to what is done for link colors that are set through preferences: http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#148

Can you update your patch to use this parsing?
(Assignee)

Comment 26

6 years ago
(In reply to Jared Wein [:jaws] from comment #25)

> Can you update your patch to use this parsing?

Yes. Give me a few hours please.
(Assignee)

Comment 27

6 years ago
Created attachment 597719 [details] [diff] [review]
Proposed fix #2 in answer to :jaws

Same fix with parser check in answer to comment 25. Built and tested
on OS X 10.6.5 and 10.7.3 with m-c revision d45c7d7b0079.
Works just as expected and seems to me a good compromise, as said earlier.

Total time spent here excluding a build from scratch to test the feature:
less than 3 minutes...
Attachment #597420 - Attachment is obsolete: true
Comment on attachment 597719 [details] [diff] [review]
Proposed fix #2 in answer to :jaws

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

Sorry, I forgot to mention before that we should add this preference to all.js so that it won't need to be manually added to about:config.

See this patch for an example of what is needed: https://bug723716.bugzilla.mozilla.org/attachment.cgi?id=594115

feedback+ with the aforementioned addition.
Attachment #597719 - Flags: feedback+
(Assignee)

Comment 29

6 years ago
Created attachment 598169 [details] [diff] [review]
Proposed fix #3 in answer to comment 28

Here it is. I tweaked a bit the name of the preference itself
to be more consistent with the browser.display.* preferences.
Final proposed name is

  browser.display.standalone_images.background_color
Attachment #597719 - Attachment is obsolete: true
Comment on attachment 598169 [details] [diff] [review]
Proposed fix #3 in answer to comment 28

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

This looks good to me. We could probably add some reftests for the pref here too using a transparent image.
Attachment #598169 - Flags: review?(smontagu)
Attachment #598169 - Flags: review?(dolske)
Attachment #598169 - Flags: feedback+
Assignee: pascalc → daniel
Status: NEW → ASSIGNED
Attachment #598169 - Flags: review?(smontagu) → review+
(Assignee)

Comment 31

6 years ago
Thanks for the review Simon. Anyone: feel free to adopt my patch and check it
in for me, I'm on vacation until next week, only partly connected through a
weak link, and I won't be available fully before end of the month.

I really think this should be checked in. The dark bg is a burden on web
developers and the hidden pref should be made public asap.
Keywords: checkin-needed

Updated

6 years ago
Attachment #584067 - Attachment is obsolete: true

Updated

6 years ago
Whiteboard: [autoland-try]

Updated

6 years ago
Whiteboard: [autoland-try] → [autoland-in-queue]

Comment 32

6 years ago
Autoland Patchset:
	Patches: 598169
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=55d56f8a8eac
Try run started, revision 55d56f8a8eac. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=55d56f8a8eac

Comment 33

6 years ago
Try run for 55d56f8a8eac is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=55d56f8a8eac
Results (out of 216 total builds):
    exception: 2
    success: 176
    warnings: 24
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-55d56f8a8eac

Updated

6 years ago
Whiteboard: [autoland-in-queue]
Comment on attachment 598169 [details] [diff] [review]
Proposed fix #3 in answer to comment 28

This overwhelmingly strikes me as a "omgchange" backlash, and at a rather minor change at that. Adding a pref is almost  never the right thing to do, and  just adds needless complexity to the code.

Or, put another way, if this is really such a big issue having a pref helps only the vanishingly small number of user who seek out and change it.

I see no fundamental reason this couldn't just be done as an addon.
Attachment #598169 - Flags: review?(dolske) → review-
(Assignee)

Comment 35

6 years ago
(In reply to Justin Dolske [:Dolske] from comment #34)
> Comment on attachment 598169 [details] [diff] [review]
> Proposed fix #3 in answer to comment 28
> 
> This overwhelmingly strikes me as a "omgchange" backlash, and at a rather
> minor change at that. Adding a pref is almost  never the right thing to do,
> and  just adds needless complexity to the code.
> 
> Or, put another way, if this is really such a big issue having a pref helps
> only the vanishingly small number of user who seek out and change it.
> 
> I see no fundamental reason this couldn't just be done as an addon.

Let's put it that way:

1. if an add-on is feasible, and because ALL USERS MATTER, the dark background
   should not have hit the trunk before the availability of that add-on.

2. Web Agencies started loving Firefox again because of the developer tools;
   the dark bg is a burden on anyone doing more than browsing PHOTOS with a
   browser. Discussed that with a few friends working in such companies. I quote
   "I don't understand why there is a no user settings for this, why do they
    do that to us?". Sorry, Justin, this is _not_ a minor change. And in terms
   of accessibility, again, it's not a minor change either.

3. the fix is an attempt to fix problem outlined in item 1. I just don't
   understand how a feature impacting so many users is not released
   simultaneously with the counter-measure for people needing it.
(In reply to Justin Dolske [:Dolske] from comment #34)

> I see no fundamental reason this couldn't just be done as an addon.

Ta-da: https://addons.mozilla.org/en-US/firefox/addon/old-default-image-style/

(In reply to Marcel Dejean from comment #18)
> Popular websites like Reddit are filled with links to standalone images.

As a heavy Reddit user myself, I'll note that the feedback I've seen has been overwhelmingly positive. As one example: http://www.reddit.com/r/firefox/comments/pbqzy/so_firefox_11_displays_photos_in_a_lightbox/
(In reply to Daniel Glazman from comment #35)
>    I just don't
>    understand how a feature impacting so many users is not released
>    simultaneously with the counter-measure for people needing it.

I think you're mistaken that it's impacting any significant number of users or that we have to block making changes on the availability of addons to undo them.

I also suspect that this isn't likely to be a useful line of debate for either of us! :)
(Assignee)

Comment 38

6 years ago
(In reply to Justin Dolske [:Dolske] from comment #36)
> (In reply to Justin Dolske [:Dolske] from comment #34)
> 
> > I see no fundamental reason this couldn't just be done as an addon.
> 
> Ta-da:
> https://addons.mozilla.org/en-US/firefox/addon/old-default-image-style/

Excellent! Now, how do users find it rapidly when they discover the behaviour
of their Firefox changed after last update and dislike the new one?

Comment 39

6 years ago
(In reply to Daniel Glazman from comment #38)
> (In reply to Justin Dolske [:Dolske] from comment #36) 
> > Ta-da:
> > https://addons.mozilla.org/en-US/firefox/addon/old-default-image-style/
> 
> Excellent! Now, how do users find it rapidly when they discover the behaviour
> of their Firefox changed after last update and dislike the new one?

You seem to care a lot about these many many users who will be horribly broken by this outrageous change; I hope you'll do your best to get the word out to them.  

Also, kind of a jerk move to propose a solution and then call it insufficient when someone builds it for you.
(Assignee)

Comment 40

6 years ago
(In reply to Asa Dotzler [:asa] from comment #39)

> Also, kind of a jerk move to propose a solution and then call it
> insufficient when someone builds it for you.

Were we waiting for you, Asa? Remind me who wanted to get
rid of all developer tools including the js console a few years ago?
Web Agencies are always early adopters; please them and they'll
recommend your browser to customers; piss them off and they'll
switch to another environment and recommend it too. Oh, right,
I forgot you don't care at all about corporate users... My bad.

I said "Excellent!" and I did mean it.

I won't even comment on "jerk move", just deserves a contemptuous smile
until you write c++ patches yourself.

Justin: again, I said "Excellent!". Let's close this bug as WONTFIX
then and I'll make sure to give some visibility to the add-on.
Could be good to have AMO and planet do the same at least once.

Side note: I still think this should be offered via a real preference
editable through the preferences panel and available builtin and not
through an add-on. The add-on is a compromise people can certainly live
with in the meantime, let's see how they react?
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 41

6 years ago
(In reply to Daniel Glazman from comment #40)

> Side note: I still think this should be offered via a real preference
> editable through the preferences panel and available builtin and not
> through an add-on.

For instance in Preferences > Content > Colors, a perfect fit.

Comment 42

6 years ago
In the options panel, there is a section for Fonts & Colours, to implement a change that affects fonts and/or colours and not provide the user with an option seems like an oversight, to then justify that is simply ignorant. Give users control or remove the whole section. But whatever the decision is, this bug should fall in line with the rest of the product.
Removed the check in request because WONTFIX'ed.
Keywords: checkin-needed
So wait.  Hold on.

This bug started out being about using a white background for the _image_ itself (not to be confused with the body; this matters for transparent images).

Then as far as I can tell it got hijacked, a bunch of bitching back and forth happened, and it got wontfixed in the "we won't add a pref for the body background" sense.

Was the bug as originally filed wontfix as of comment 7?  Or is it still worth considering using a white or default background for the _image_ itself?  For opaque images it, again, makes no difference; the question is whether it's better for transparent images.

Another question, which would want a separate bug if it makes any sense at all, is whether an outset border or some such would help if the issue is the image seeming to blend into the dark background...

Comment 45

6 years ago
WONTFIXED and ranting. I don't like this.
I would rather make this 'gray' color more lighter like it's described in bug #717226, because when this will be fixed more of broken PNGs with alpha channel will be rendered properly and what's more will be visible and build-in option to change it won't be needed.
(In reply to Boris Zbarsky (:bz) from comment #44)

> Was the bug as originally filed wontfix as of comment 7?

Yeah. Perhaps I should have just done so then, but was trying to keep open to the possibility there was a sane and simple way to achieve greater perfection. I think we're pretty unlikely to find that now, and given the flame:benefit ratio I'm rather disinclined to do fiddle around more for the time being.

If folks want to experiment with more styles in addons that would be swell, and I'd be happy to look at the fruit from such experiments at a later date.

Updated

6 years ago
Duplicate of this bug: 736889
Duplicate of this bug: 736943
Duplicate of this bug: 737320

Comment 51

6 years ago
(In reply to Masatoshi Kimura [:emk] from comment #50)
> Duplicate of this bug: 737320

Why? What did the two have to do with each other? How is “Render standalone images against user-specified background” a duplicate of “Transparent images should be rendered against the default background color.” Good grief.

Comment 52

6 years ago
(In reply to Gingerbread Man from comment #51)
> (In reply to Masatoshi Kimura [:emk] from comment #50)
> > Duplicate of this bug: 737320
> 
> Why? What did the two have to do with each other? Good grief.

It's an artifact of how this "bug tracker" offers very limited ways of associating bugs with each other. "Duplicate" in this clusterhug of a bug here means "related bug, lol".

It's encouraged somewhere above to file disjoint bugs with any discontent you have to separate it out from the noise in the comments.

Comment 53

6 years ago
(In reply to Asa Dotzler [:asa] from comment #17)
> Viewing stand-alone images is already and edge case

This seems like a ridiculous assertion. The web isn't 100% lightboxes, whatever your personal preference for web design may be. Image galleries have existed for longer than I've browsed the web. Whether left-clicking thumbnails to open them in the same tab full size, or left-clicking them to open them in a new tab via the target="_blank" attribute, or right-clicking and choosing "Open In New Tab", or middle-clicking the thumbnails, the scenario is not even remotely uncommon.

(In reply to Daniel Glazman from comment #38)
> Excellent!

I'm sorry to say I don't share your enthusiasm. Having to install yet another extension for a feature that was removed for no reason is absurd — even if making said extension is an act of kindness that requires time and effort.

I would find it halfway acceptable if this was fixable by a user style, but as far as I can tell it's not possible to apply a style when the URL for the standalone image doesn't end in an image extension. This is the case for picture attachments on boards powered by the popular vBulletin, and miscellaneous other sites. A discussion on this is at
http://forums.mozillazine.org/viewtopic.php?p=11840051#p11840051

(In reply to Daniel Glazman from comment #41)
> For instance in Preferences > Content > Colors, a perfect fit.

So an entirely new preference adds undue “code complexity”. What's the case against simply using the existing preference, as Firefox has done all these years? How complex is it to throw an IF check in there?
Why this was WONTFIXED? It looks like this bug was hijacked like Boris Zbarsky said

https://bugzilla.mozilla.org/attachment.cgi?id=593357
is still unwatchable with others transparent images...
any ideas to repopen this bug or I will create in meantime another one for this issue...

Updated

6 years ago
Duplicate of this bug: 739017

Comment 56

6 years ago
Please consider reopening this bug.

As Alex Faaborg noted in bug 376997 comment 23:

> I think this approach works well for video, which is meant to be immersive,
> and sort of viewed in a theater environment.  But I'm not as sure about
> images.  Some of them are fantastic photography which you want to view in
> that theater environment.  But some are sections of web pages that have an
> alpha channel and the user's goal isn't to view them as art much as to
> extract them from the page.

Please have a bit more respect for the status quo.  Every previous version of Firefox and all other browsers I'm aware of display standalone images in the top left, on a white background.  Did it occur to any of the FF devs that some people might like it that way?

Provide an alternative viewing mode? Sure. Make that new mode the default? If you must. But to radically change the display of standalone images without providing an option for the traditional behaviour (even in about:config)?  To expect users to install an Add-on to get the old behaviour back?  That's just not reasonable.

Comment 57

6 years ago
If you don't want people to rant and flame, then don't violate the very core concepts of Firefox itself. Firefox is the browser that is designed for the users. Every major UI change has always provided a way to opt out in the main code. I mean, you can still opt into the Firefox 3 UI, and this is Firefox 11. 

Furthermore, the behavior is actually broken. Transparent PNGs are built with the idea of a light colored background. To not display them correctly is not just a user preference problem, but an actual bug. In fact, I plan on filing a separate bug for this single behavior which is not acceptable at all. Transparent PNGs when viewed by themselves are properly displayed with either a white, light grey, or checkered background.

I mean, you've broken Wikimedia commons/Wikipedia's images so that you cannot see the full sized image. This is not acceptable.

Comment 58

6 years ago
Sorry for the spam, but forgot something above: One the reasons for having these options to restore the old behavior is so that when a bug shows up, people have the option to swap back while you fix said bug. You should not have been caught flatfooted with this and had to create an addon, especially with such a trivial change. (It's just a stylesheet change in the omnijar, right?) 

In my opinion, this is just a basic part of program design, especially when there is a fast turn around. When you have a new feature, step one is to implement it as an option, while the kinks are worked out. Step two is to make it the default, which will get you feedback about the change. Step three is to remove the option to disable this.

Now, of course, sometimes this ideal is not practical, because the new feature is a complete code change, and redesigning a new version of old features can be taxing. But this was not the case here.

So maybe it's not the Firefox is betraying its design principals as much as you still aren't adjusted to the fast turn around time. Either way, this is something that needs to be improved.

Oh, and when I file a new bug to deal with what Boris is talking about, to get away from the ranting in this bug, please don't close it as a duplicate. Fixing the background of the image portion by default is not the same thing as providing an opt out for the new theater mode.

Comment 59

6 years ago
I have now filed Bug 743474 per my statements above.
Duplicate of this bug: 743474

Comment 61

6 years ago
Just to clarify the title of this bug meant the "default background color" as set by a setting found in about:config, not e.g. an info embedded in the image like the bKGD chunk in PNG images?

Comment 62

6 years ago
I'm surprised that this bug has generated such heated discussion, and even more surprised that it has been wontfixed.

In response to comment 2, and as noted in comment 56, the default behaviour in all major browsers has been a to render images against a white background.  The result of this is that there are now many images in the world that - intentionally or otherwise - rely on this, because they've never been viewed against anything else.  I have come across several from different sources in the last couple of weeks.

As a result, the implementation of Bug 376997 has lead to a regression in behaviour for users looking at these images.

The way to fix this seems really straightfoward: retain the centering and default grey background of Bug 376997, but make the background of the image itself white (or whatever the user-specified default is).

This is my understanding of the proposal in Comment 0, and provides all the benefits of Bug 376997, but restores the "transparent means white" behaviour that, for better or worse, people have come to expect.

I can't see any valid reason why this bug has been wontfixed, although there do seem to have been some misunderstandings of the proposed fixed.

Comment 63

6 years ago
the point is that there is already a preference specifying what background the user wants, aka "browser.display.background_color".

why was it not used in bug 376997 ? why was it hardcoded in "omni.ja:res/TopLevelImageDocument.css" ? that should never have been allowed. how did that slip through code review ?

one of the advantages of firefox over other browsers is, that everything is configurable in "about:config".
(In reply to Bob Smith from comment #63)
> the point is that there is already a preference specifying what background
> the user wants, aka "browser.display.background_color".

AFAIK, that pref controls also the default background of all web pages. So, if you set it to red and a web page doesn't explicitely specify background-color:white, you'll still get a red background.

Comment 65

6 years ago
(In reply to Bob Smith from comment #63)
> the point is that there is already a preference specifying what background
> the user wants, aka "browser.display.background_color".

It's been brought up before, e.g. bug 737320

> why was it hardcoded in "omni.ja:res/TopLevelImageDocument.css" ? that
> should never have been allowed. how did that slip through code review ?

It didn't get past anyone. See bug 376997 comment 88.

Comment 66

6 years ago
(In reply to Justin Dolske [:Dolske] from comment #34)
> Or, put another way, if this is really such a big issue having a pref helps
> only the vanishingly small number of user who seek out and change it.

A preference in about:config would be an improvement on the status quo.

It would be even better to have a discoverable, non warranty-voiding preference.

The best solution would be to render images with alpha channels against the default background color, as FF always used to prior to the fix for bug 376997, regardless of whether that image is then surrounded by a dark grey background.

> I see no fundamental reason this couldn't just be done as an addon.

From the user's perspective that's worse than a preference hidden in about:config.  People still need to "seek out" the fix, but they have the hassle and risk of installing an add-on (which might not even be possible in locked-down environments).

This is a "big issue".  If you force users to go searching for fixes, some of them will decide it's easier to use a different browser instead.


(In reply to pdw from comment #62)
> The way to fix this seems really straightfoward: retain the centering and
> default grey background of Bug 376997, but make the background of the image
> itself white (or whatever the user-specified default is).

The distinction between the background for the page and the image itself was highlighted by Boris in comment 44, and Terrell Kelley (comment 58) spun it out into bug 743474, though that bug suggests hard-coding to "a light color (greater than #CCCCCC)".

I agree that it would be better for this to be configurable (indeed, this is essential if we're to properly correct the regression).

I'm not opposed to Daniel Glazman's suggestion that we have two preferences, as long as they interact sensibly:

1) browser.display.background_color (existing)

2) browser.display.standalone_images.background_color (new; see comment 24 and comment 29)

In order to restore the display behaviour for transparent images that existed prior to the fix for bug 376997, browser.display.standalone_images.background_color must default to browser.display.background_color.

I haven't tried Daniel's patch, so I'm not sure whether it applied to the image itself, or the entire page (overriding the dark grey background).

It seems sensible to have it just apply to the image itself, though I think there should be a separate preference that disables lightbox mode altogether (i.e. kills both the centering and the grey page background, making things exactly as they used to be).

We could have yet another preference to control which color is used for the lightbox background (so people can choose their favourite shade of dark grey), but I don't think that's really necessary.

The most important thing is to get the behaviour right by default, so users don't have to muck around with preferences and add-ons.  That means restoring the behaviour that we had prior to the fix for bug 376997.

Comment 67

6 years ago
Since there is afaik no pseudo-element / pseudo-class to choose only standalone image documents it's impossible to change the page's background color via Stylish or userContent.css.

By using body > img:only-child it's possible to style the image though (with a very small, probably negligible, chance of false positives in other image galleries).

(In reply to Adam Nielsen from comment #12)
> What about making it so that if you click on the image it switches between
> the default background and the default foreground?  Those colours can be
> customised already and are usually opposite light/dark, so making it easy to
> toggle between those two I think would be useful.

Clicking is already used for resizing (besides it's not possible to use this with CSS), but with :hover

body > img:only-child {
   background-color: -moz-default-color;
}
body > img:only-child:hover {
   background-color: -moz-default-background-color;
}

(In reply to pdw from comment #62)
> The way to fix this seems really straightfoward: retain the centering and
> default grey background of Bug 376997, but make the background of the image
> itself white (or whatever the user-specified default is).

always white:
body > img:only-child {
   background-color: white;
}

always default background:
body > img:only-child {
   background-color: -moz-default-background-color;
}

default background on hover only:
body > img:only-child:hover {
   background-color: -moz-default-background-color;
}

(In reply to James May [:fowl] from comment #8)
> subtle checkerboard?
(In reply to Andreas Jung from comment #9)
> While checkerboard is a good way to make half-transparent parts easier to
> recognize it doesn't look very nice.

subtle checkerboard on hover (that's what Wikipedia does):
body > img:only-child:hover {
   background-image: url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAAAAAA6mKC9AAAAGElEQVQYV2N4DwX/oYBhgARgDJjEAAkAAEC99wFuu0VFAAAAAElFTkSuQmCC");
}

Comment 68

6 years ago
(In reply to Andreas Jung from comment #67)
> Since there is afaik no pseudo-element / pseudo-class to choose only
> standalone image documents it's impossible to change the page's background
> color via Stylish or userContent.css.

Bug 626778 / Bug 626776 propose a solution for that problem.

Comment 69

6 years ago
(In reply to Andreas Jung from comment #67)
> Since there is afaik no pseudo-element / pseudo-class to choose only
> standalone image documents it's impossible to change the page's background
> color via Stylish or userContent.css.

For a hacky workaround see bug 736270 comment 7, which styles the body on images where the URL matches *.(jpg|gif|png). That'll work for images such as

http://www.mozilla.org/media/img/firefox/template/header-logo.png

but not

https://bug713230.bugzilla.mozilla.org/attachment.cgi?id=584109

As you say, until bug 626778 gets fixed I don't think there's a foolproof way to do it.

Comment 70

6 years ago
All well and good, but as noted in comment 66,

"The most important thing is to get the behaviour right by default, so users don't have to muck around with preferences and add-ons.  That means restoring the behaviour that we had prior to the fix for bug 376997."

I think this bug should focus on that (i.e. what it says in the summary: "Transparent images should be rendered against the default background color").

Also see bug 743474 comment 9 - a large number of actual users have been inconvenienced out of concern for mythical users (comment 2) who were deeply upset by the use of the default background color for transparent images, but didn't complain over all these years.
Bug 754133 should be of interest to the people in this bug.

Updated

6 years ago
Duplicate of this bug: 797087

Comment 73

6 years ago
(In reply to Daniel Glazman from comment #38)
> (In reply to Justin Dolske [:Dolske] from comment #36)
> > (In reply to Justin Dolske [:Dolske] from comment #34)
> > 
> > > I see no fundamental reason this couldn't just be done as an addon.
> > 
> > Ta-da:
> > https://addons.mozilla.org/en-US/firefox/addon/old-default-image-style/
> 
> Excellent! Now, how do users find it rapidly when they discover the behaviour
> of their Firefox changed after last update and dislike the new one?

That's easy.

① File a bug report about the loss of the old display-it-at-the-top-left behaviour and the lack of configurability of the new, broken, behaviour.

② Get the bug report marked as a duplicate of this one.

③ Read the comments here.

④ Find the link to the extension...

⑤ Install it, find that it does the job.

Seems to me that that extension, or its functionality, should be built in.
You need to log in before you can comment on or make changes to this bug.