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

RESOLVED FIXED in mozilla12

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Greg Karz, Assigned: Greg Karz)

Tracking

Trunk
mozilla12
Points:
---

Firefox Tracking Flags

(firefox11 affected)

Details

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

Attachments

(3 attachments)

(Assignee)

Description

5 years ago
Created attachment 583206 [details]
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
(Assignee)

Updated

5 years ago
Component: Page Info → General
OS: Windows 7 → All
Hardware: x86 → All
(Assignee)

Updated

5 years ago
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

Updated

5 years ago
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

Updated

5 years ago
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
status-firefox11: --- → affected
Version: 11 Branch → Trunk
(Assignee)

Comment 2

5 years ago
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

5 years ago
(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".
(Assignee)

Comment 4

5 years ago
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)
Attachment #583679 - Flags: approval-mozilla-beta?
Created attachment 583683 [details]
Screenshot of patch
Attachment #583683 - Flags: ui-review?(shorlander)
(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
Attachment #583679 - Flags: approval-mozilla-beta?
(Assignee)

Comment 7

5 years ago
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

5 years ago
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)

Updated

5 years ago
Attachment #583679 - Flags: feedback?(dao) → feedback+
Attachment #583679 - Flags: review?(roc) → review+
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
Last Resolved: 5 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

Updated

5 years ago
Duplicate of this bug: 741530
You need to log in before you can comment on or make changes to this bug.