712347 - "The image ... cannot be displayed because it contains errors." error message is hard to read on the dark background (imageDocument)

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Greg Karz]

Attachment: image.png

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:11.0a1) Gecko/20111220 Firefox/11.0a1
Build ID: 20111220031159

Steps to reproduce:

Open a malformatted image


Actual results:

"The image "..." cannot be displayed because it contains errors." message is hard to see on the dark background


Expected results:

It would be nice to have a lighter color for that text

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

In the case of an image decoding error, error text is set as the "alt" text of the image. This alternative text is then displayed as the image.

It appears that we can fix this by setting a CSS |color| property on the |img| tag.

In /layout/style/TopLevelImageDocument.css, we may want to add the following inside the img rule:

img {
 color: #eee;
 ... other rules
}

Additionally, we may want to set some default font-families for this text. The current implementation doesn't allow for platform-specific themes, so we may want to wait, but it could be worth an investigation.

Comment 2

[Greg Karz]

Sorry to ask this here (couldn't get help on the irc), I have the firefox sources from mozilla-central, I added the color: #eee; to the |img| tag. What should be my next actions? How do I add it to mozilla-central? Or what is the procedure now? I would really like to help fix bugs.

Comment 3

[Paul Rouget [:paul]]

(In reply to Greg Karz from comment #2)
> Sorry to ask this here (couldn't get help on the irc), I have the firefox
> sources from mozilla-central, I added the color: #eee; to the |img| tag.
> What should be my next actions? How do I add it to mozilla-central? Or what
> is the procedure now? I would really like to help fix bugs.

Great! First, you need to build Firefox. Read this: https://developer.mozilla.org/En/Simple_Firefox_build

Then, you need to understand how to hack Firefox. Read this: https://developer.mozilla.org/en/Hacking_Firefox

Your goal is to create a patch and post it here.

I strongly encourage you to join us on IRC and ping "jaws".

Comment 4

[Greg Karz]

Attachment: Added color:#eee; and text-align:center;

I added text-align:center so that the message is centered as an image would be. Is this a bad idea? (please warn me if I did something wrong)

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot of patch

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Greg Karz from comment #4)
> Created attachment 583679 [details] [diff] [review]
> Added color:#eee; and text-align:center;
> 
> I added text-align:center so that the message is centered as an image would
> be. Is this a bad idea? (please warn me if I did something wrong)

Nice job on your first patch! I've added a screenshot of it to this bug and requested UI-review from Stephen Horlander on the design.

I'm not sure if I like the text-align: center, but the lack of margins on the body push me towards liking text-align: center.

Also, I don't think this bug exists in the beta channel. The general way that we approach bugs in release/beta/aurora, is to get the bug fixed in trunk (mozilla-central), and then request approval for whatever branches that are also affected.

Comment 7

[Greg Karz]

Thanks! And btw, what is your IRC channel, or how can I access it (sry, I'm a newbie to IRC too) and how can I create a patch for another bug, so that it doesn't contain the fix in this patch?

Comment 8

[Josh Matthews [:jdm]]

Either #introduction or #developers would be fine choices. For working on multiple patches, learning how to use Mercurial Queues (https://developer.mozilla.org/en/Mercurial_Queues) is strongly encouraged. #introduction is a great place to ask followup questions about general development practices that aren't related to a particular bug.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

There is also more information about IRC here: https://wiki.mozilla.org/IRC

The IRC server is located at irc.mozilla.org

Comment 10

[Stephen Horlander [:shorlander]]

Comment on attachment 583683 [details]
Screenshot of patch

This looks good given the constraints. 

We should probably consider something nicer at some point though :)

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 583679 [details] [diff] [review]
Added color:#eee; and text-align:center;

I think we should add a comment to these lines of CSS so people understand why we have set a color and text-align on the img, but other than it "looks good to me!".

I've requested feedback from Dao to get his input on the CSS change and review from :roc since this is in /layout

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Greg: Can you please follow the steps in Marco's blog post about readying your patch for check-in? http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Greg: Do you have any questions about making the patch ready for check-in? I'll go ahead and fix it up on Friday if there isn't any movement here.

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/7f728aef5ad8

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Congratulations on your first patch Greg!

Comment 16

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/7f728aef5ad8