Closed Bug 39210 Opened 24 years ago Closed 22 years ago

document.bgcolor reported as white instead of #ffffff

Categories

(Core :: DOM: Core & HTML, defect, P3)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Future

People

(Reporter: jruderman, Assigned: caillon)

References

(
URL
)

Details

(Keywords: dom0, testcase)

Attachments

(3 files)

readable and slightly simplified version of original page
2.21 KB, text/html
Details
test case that just shows the 'white' problem
123 bytes, text/html
Details
Fix 1.0
1.54 KB, patch
sicking
: review+
jst
: superreview+
Details | Diff | Splinter Review 
if you have <body bgcolor="white">, mozilla tells javascript that the bgcolor 
is "white" instead of "#ffffff".  i don't know which is more correct, but ie 
and ns4x both say "#ffffff".

note: the url given doesn't work at all unless you replace document.all.ds with 
document.getElementById('ds').  i'll attach a readable, working, and partially 
simplified version of the page, and then i'll attach a testcase.
several other bugs have been reported with the same url.  another known bug, 
bug 38296, shows up by showing some style information at the top of the 
original page (but not the "slightly simpified" version).
OS: other → Windows 98
Hardware: Other → PC
This bug has been marked "future" because the original netscape engineer
working on this is over-burdened. If you feel this is an error, that you or
another known resource will be working on this bug,or if it blocks your work
in some way -- please attach your concern to the bug for reconsideration.
OS: Windows 98 → All
Hardware: PC → All
Target Milestone: --- → Future
Keywords: testcase
Status: NEW → ASSIGNED
Keywords: dom0
I don't see any function to translate between a color string and hex.
There is
string->rgb
hex->rgb
but not string->hex 
nor rgb->hex.
Relevant code : 
nsHTMLValue value(styleColor->mBackgroundColor);
ColorToString(value, aBgColor);

This code apparently just takes the attribute value in the body tag and returns
it. I'm a little stuck here...
Fabian, if you're programming in Mozilla you could use NS_ColorNameToRGB which 
returns a PRBool if it succeeds.

   succeeded = NS_ColorNameToRGB(colorStr, &color)
Attached patch Fix 1.0 — — Splinter Review 
So assuming this bug is valid, this patch fixes it.
Comment on attachment 90398 [details] [diff] [review]
Fix 1.0

>Index: content/html/content/src/nsHTMLBodyElement.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/content/html/content/src/nsHTMLBodyElement.cpp,v
>retrieving revision 1.95
>diff -u -r1.95 nsHTMLBodyElement.cpp
>--- content/html/content/src/nsHTMLBodyElement.cpp	22 May 2002 23:59:43 -0000	1.95
>+++ content/html/content/src/nsHTMLBodyElement.cpp	7 Jul 2002 00:28:23 -0000
>@@ -559,7 +559,11 @@
> NS_IMETHODIMP 
> nsHTMLBodyElement::GetBgColor(nsAString& aBgColor)
> {
>-  nsresult rv = GetAttr(kNameSpaceID_None, nsHTMLAtoms::bgcolor, aBgColor);
>+  aBgColor.Truncate();
>+
>+  nsAutoString attr;
>+  nscolor bgcolor;
>+  nsresult rv = GetAttr(kNameSpaceID_None, nsHTMLAtoms::bgcolor, attr);

I would just assign directly into aBgColor since you are guarenteed to get an
empty string back if the function returns NS_CONTENT_ATTR_NOT_THERE.

Either way, r=sicking
Attachment #90398 - Flags: review+
I guess that would work too.  The reason I didn't assign directly to the out
parameter though, is that the value we get right there is not necessarily the
value we want to use for the out parameter, though it may well be so.  In fact,
we only want the value we get from the attribute in 1 out of 3 cases...

I'll switch if Johnny wants me to do so, but I still think the way I have it
done is more readable.

Taking this bug...
Assignee: jst → caillon
Status: ASSIGNED → NEW
Comment on attachment 90398 [details] [diff] [review]
Fix 1.0

sr=jst
Attachment #90398 - Flags: superreview+
Checked in
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Verifying this based on stummala's comments in bug 168346.
Status: RESOLVED → VERIFIED
*** Bug 187744 has been marked as a duplicate of this bug. ***
reopening per recent dup, resetting milestones, etc.
Status: VERIFIED → REOPENED
Keywords: regression
Priority: P3 → --
Resolution: FIXED → ---
Target Milestone: Future → ---
That bug has nothing to do with this one, and even if it did this one should not
have been reopened.
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
restoring verified state.
Status: RESOLVED → VERIFIED
Unless you own the bug, don't touch priorities or milestones.  Undoing damage.
Keywords: regression
Priority: -- → P3
Target Milestone: --- → Future
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: