Closed Bug 712347 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12
Tracking Status
firefox11 --- affected

People

(Reporter: greg.karz, Assigned: greg.karz)

References

()

Details

(Whiteboard: [good first bug][mentor=jwein][lang=css])

Attachments

(3 files)

Attached image 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
Component: Page Info → General
OS: Windows 7 → All
Hardware: x86 → All
Summary: "The image "..." cannot be displayed because it contains errors." message is hard to see on the dark background → "The image "..." cannot be displayed because it contains errors." message is hard to read on the dark background
Component: General → Layout
Product: Firefox → Core
QA Contact: page.info → layout
Summary: "The image "..." cannot be displayed because it contains errors." message is hard to read on the dark background → "The image ... cannot be displayed because it contains errors." error message is hard to read on the dark background (imageDocument)
Version: 11 Branch → 12 Branch
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Whiteboard: [good first bug][mentor=jwein][lang=css]
Version: 12 Branch → 11 Branch
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.
(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".
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)
Attachment #583679 - Flags: approval-mozilla-beta?
(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.
Assignee: nobody → greg.karz
Status: NEW → ASSIGNED
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?
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.
There is also more information about IRC here: https://wiki.mozilla.org/IRC

The IRC server is located at irc.mozilla.org
Comment on attachment 583683 [details]
Screenshot of patch

This looks good given the constraints. 

We should probably consider something nicer at some point though :)
Attachment #583683 - Flags: ui-review?(shorlander) → ui-review+
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
Attachment #583679 - Flags: review?(roc)
Attachment #583679 - Flags: feedback?(dao)
Attachment #583679 - Flags: feedback?(dao) → feedback+
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
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.
https://hg.mozilla.org/integration/fx-team/rev/7f728aef5ad8
Whiteboard: [good first bug][mentor=jwein][lang=css] → [good first bug][mentor=jwein][lang=css][fixed-in-fx-team]
Congratulations on your first patch Greg!
https://hg.mozilla.org/mozilla-central/rev/7f728aef5ad8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=jwein][lang=css][fixed-in-fx-team] → [good first bug][mentor=jwein][lang=css]
Target Milestone: --- → mozilla12
Duplicate of this bug: 741530
You need to log in before you can comment on or make changes to this bug.