Closed
Bug 38370
Opened 25 years ago
Closed 21 years ago
Can't change <HR> and <BR> color
Categories
(Core :: Layout, defect, P3)
Core
Layout
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: supersamat, Assigned: caillon)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [Hixie-P1] [fix almost in hand] BLOCKED)
Attachments
(6 files, 7 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
In html.css, the HR is defined to be 1px wide and have a color of -moz-bg-inset.
Since color: or background-color: (on thinner HRs) does not work on an HR,
border-color: should change the -moz-bg-inset color to another color.
My pre-M16 nightly does not seem to let background-color: change anything with
the HR. Setting background-color: does change the shade of the border, but
border-color: doesn't. Attached testcase demonstrates (with workaround).
Also, using border-color: to set the color of an HR is a bit illogical... Yes,
HR is a box element (I think) and is rendered that way and that is the way it
has to be done. Web developers unfamiliar with how Mozilla works may not know to
use it though. With IE, it lets you simply use a color: to assign a color to the
HR. This is much more logical; if I wanted to make a 2px high HR that is black,
in IE I could write: "hr { height: 2px; color: black; }". Perhaps something with
how Mozilla handles HRs may be useful?
Reporter | ||
Comment 1•25 years ago
|
||
Reporter | ||
Comment 3•25 years ago
|
||
Yeah, forgot to mention it may be related to 2590... HRs can be styled using a
full border style declaration (ex border: solid 1px black) but this causes
problems in Nav4. border-color: (which should work anyway) or color: would be
nicer to use.
Comment 4•25 years ago
|
||
It's a rendering problem: reassigned to buster.
See bug 38370 for related comments.
Assignee: pierre → buster
Status: UNCONFIRMED → NEW
Component: Style System → Layout
Ever confirmed: true
Summary: Can't change HR color → Can't change HR and BR color
Comment 5•25 years ago
|
||
The fix for this bug should be relatively simple. At the moment, we are treating
HR and BR as special cases -- in the case of BR, this is completely unnecessary,
and in the case of HR, it is only required for making sure that the element does
not overlap floats (ok, so that is not easy, but it is all that needs doing).
Here is a summary of possible ways of implementing these two elements in a
spec compliant way, while keeping full backwards compatibility:
== BR ==
Simply define BR as...
BR:before { display: inline; content: "\A"; }
...and then treat BRs the same way as empty DIVs. Kipp and I discussed how
BRs affect the lines they are in, and I drew some diagrams which I will insert
here too:
abc<br><br><br>def
...renders as:
+---------------------------------------+
| +-------+ + |
| | a b c | | |
| +-------+ + |
| + |
| | |
| + |
| + |
| | |
| + |
| +-------+ |
| | d e f | |
| +-------+ |
+---------------------------------------+
(where the lines are construction lines, only designed to show where
the boxes are.)
Also:
<img><br>def
...renders as
+---------------------------------------+
| +------------+ |
| | IMG with | |
| | v-align | |
| | bottom | |
| | and inline | + |
| | display | | |
| +------------+ + |
| +-------+ |
| | d e f | |
| +-------+ |
+---------------------------------------+
Related pages:
http://www.bath.ac.uk/%7Epy8ieh/internet/projects/mozilla/br.html
http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/hrbrstyles.html
bug 4247 - Empty inline elements are not affecting the inline box model
bug 4248 - Trailing spaces on inline elements wrapping @ boundary badly treated
bug 2590 - <HR> and <BR> are unstylable!!!
bug 26996 - </font><br><font> not fixed by temporary bug-24186 fix
bug 15428 - line-height values less than 1 are wrong (see comments near end)
== HR ==
This is more complicated, because the attributes do not map directly onto
the CSS model. Again, the HR can be treated as an empty primitive element,
except that some attribute mapping must be performed, and the width must be
based on available width minus the width of floats. For each attribute, here
is the mapping I would recommend (applied in this order):
/* default ua.css */
hr { border-style: 1px -moz-bg-inset; margin: auto; display: block; }
/* noshade */
#thishr { -moz-border-radius: 100%; border: none;
background: -moz-shade-bg; }
/* align=left */
#thishr { margin-left: 0; margin-right: auto; }
/* align=right */
#thishr { margin-left: auto; margin-right: 0; }
/* align=center */
#thishr { margin-left: auto; margin-right: auto; }
/* size */
#thishr { height: <value of size attribute>; }
/* width */
#thishr { width: <value of width attribute>; }
/* color */
#thishr { color: <value of color attribute>;
background: <value of color attribute>; }
The following mozilla specific keywords apply:
-moz-bg-inset (<border-style> keyword)
Already implemented. Emulates nav4's 3D border style for HRs (whatever
that is).
-moz-border-radius (property)
Already implemented. Causes background and borders to be rounded.
-moz-shade-bg (suggested <color> value for 'background' property)
Should emulate the nav4 colour selection code for noshade style HRs,
whatever that is.
Note that since HR is a block-level element, the font-size will not have
any effect on rendering.
Related pages:
http://www.bath.ac.uk/%7Epy8ieh/internet/projects/mozilla/hr.html
http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/hrbrstyles.html
bug 2590
Comment 6•25 years ago
|
||
Changing BRs is not just that simple. One must also be careful of the
quirks-mode stuff for dealing with BR. Do we support ' content: "\n"; '?
Should we?
Comment 7•25 years ago
|
||
Oops, minor error on my part. BR should be
BR:before { display: inline; content: "\A"; white-space: pre; }
...note the white-space declaration.
We support content: "\A", is that what you meant?
There are some issues with <br> and quirks mode, yes (as you know, since you
wrote most of that code...). The above only applies to strict mode, it almost
certainly requires tweaking to "work" in quirks mode.
Updated•25 years ago
|
Summary: Can't change HR and BR color → Can't change <HR> and <BR> color
Updated•24 years ago
|
QA Contact: chrisd → py8ieh=bugzilla
Comment 9•24 years ago
|
||
Refering back to the orriginal post...."background-color:" sets the fill color
of a HR and not "color:"
Is this the way it should be?
Comment 10•24 years ago
|
||
Taking a stab at prioritizing buster's nsbeta3 bugs...
Whiteboard: [nsbeta3-]
Comment 11•24 years ago
|
||
We'll do this for the next release. It's a minor issue compared to the other
things we need to fix for rtm.
Target Milestone: --- → Future
Comment 12•24 years ago
|
||
Ian:
You write:
'Simply define BR as...
BR:before { display: inline; content: "\A"; }'
but you seem to have missed several explanations as to why this doesn't work,
and, in fact, breaks either (a) countless existing pages or (b) Mozilla's clear
implementation.
Oops.
Anyway:
http://lists.w3.org/Archives/Public/www-style/2000Mar/0006.html
<BR clear="left">
Clear doesn't apply to inline-level elements. [So BR can't be
inline-level)
[And there are others that I can't find at the moment.]
Comment 13•24 years ago
|
||
At the W3C face to face meeting this week, it was mentioned that we could even
do the floats thing, by using -moz-float-edge. So fixing this bug may actually
cover _all_ the bases for HR and BR.
Updated•24 years ago
|
Keywords: mozilla0.9
Comment 15•24 years ago
|
||
Asa: Why do you consider this bug to be important for 0.9?
Comment 16•24 years ago
|
||
removing nomination, misread of the report. please excuse the spam.
Keywords: mozilla0.9,
nsbeta1
Updated•24 years ago
|
Keywords: mozilla1.0
Whiteboard: [nsbeta3-] → (py8ieh: ho hum)
Comment 17•24 years ago
|
||
Buster: Are you actually doing some work on this right now?
Comment 18•24 years ago
|
||
nada, as indicated by the "future" milestone
Comment 19•24 years ago
|
||
Taking bug. I have a patch which fixes this, modulo dependencies on bug 69491
and bug 69518. I shall attach it presently, but checking it in without fixing
the other two bugs would break backwards compatability.
Nevertheless, I would appreciate it if people could look over the patch and
give me any comments suggesting possible improvements. David? Marc?
Assignee: buster → ian
Keywords: patch
OS: Windows 98 → All
QA Contact: ian → dbaron
Hardware: PC → All
Whiteboard: (py8ieh: ho hum)
Target Milestone: Future → mozilla1.0
Comment 20•24 years ago
|
||
Comment 21•24 years ago
|
||
I'll review the changes. It will take a while as they are pretty extensive. BTW:
since when does Hixie write code? We are all in trouble now ;)
Comment 22•24 years ago
|
||
Hmm, just found another bug that is blocking the checkin of this fix. So far,
the following bugs are causing problems that block this fix from going in:
bug 69398: label:after {content: ":"} repeats creation of : with mouse movement
bug 69491: -moz-float-edge sucks with auto margins and left floaters
bug 69518: br:before stiffles dynamic changes of the clear attribute
Whiteboard: BLOCKED
Comment 23•24 years ago
|
||
BTW, I'm keeping this live in my tree through all the massive changes, so if
the patch attached to this bug stops working at any point just ask me to create
a new one.
Updated•24 years ago
|
Whiteboard: BLOCKED → [fix in hand] BLOCKED
Updated•24 years ago
|
Whiteboard: [fix in hand] BLOCKED → [Hixie-P4] [fix in hand] BLOCKED
Comment 24•24 years ago
|
||
Please test the performance consequences of doing this, especially on long
pages.
Comment 25•23 years ago
|
||
waterson: this is the HR bug you asked about. The patch no longer applies
because hyatt rewrote the attribute mapping code, but in theory it should be
possible to update it. The only problem then is the four blocking bugs, all of
which are serious in their own right anyway.
Comment 26•23 years ago
|
||
Well, this bug covers _both_ <hr> and <br>; do they need to be conflated? (Seems
like we could fix one without the other...)
Comment 27•23 years ago
|
||
They can be fixed separately, yes. The fixes are quite similar (as my now
defunct patch demonstrated).
Comment 28•23 years ago
|
||
*** Bug 91885 has been marked as a duplicate of this bug. ***
Comment 29•23 years ago
|
||
Reassigning to waterson.
waterson: In theory, the only thing that needs doing is to take my patch and
make it work with hyatt's rule tree change. That should only involve the first
file affected by the patch (nsHTMLHRElement.cpp). Beyond that the "only" changes
required are those to fix the bugs blocking this one.
Needless to say, the hard bit is the fixing of the blocking bugs...
Assignee: ian → waterson
QA Contact: dbaron → ian
Whiteboard: [Hixie-P4] [fix in hand] BLOCKED → [Hixie-P1] [fix almost in hand] BLOCKED
Updated•23 years ago
|
Status: NEW → ASSIGNED
Comment 30•23 years ago
|
||
*** Bug 103546 has been marked as a duplicate of this bug. ***
Comment 31•23 years ago
|
||
*** Bug 105413 has been marked as a duplicate of this bug. ***
Comment 32•23 years ago
|
||
Bug 119606 has an example which uses colors.
Updated•23 years ago
|
Target Milestone: mozilla1.0 → Future
Comment 33•23 years ago
|
||
*** Bug 142338 has been marked as a duplicate of this bug. ***
Comment 34•22 years ago
|
||
Is there any workaround for being able to set the color of an HR? If I use
background-color (the only thing that seems to work in mozilla), it breaks the
HR in Explorer (at least on a Mac). Specifically, Explorer seems to treat the
whole line as the background, not just the horizontal rule, so even if your HR
is less that 100% wide, the color goes all the way to the edges of the page. It
looks like there's no way to get it to work on both browsers since their
behavoirs are mutually exclusive. Can anyone verify or disprove this?
Comment 35•22 years ago
|
||
border-color should work... does it?
Comment 36•22 years ago
|
||
Comment on attachment 25714 [details] [diff] [review]
Patch: removes nsHRFrame and (in standard mode) nsBRFrame, and replaces them with CSS
A few comments on this:
* GetMutableStyleData is evil. The attribute mapping functions would be much
better.
* This patch doesn't remove the hacky centering code in nsLineLayout, but it
should.
* |#if 0| and "// XXX not part of build" are unnecessary. Just delete and
remove from build. :-)
I didn't read the whole thing, though...
Comment 37•22 years ago
|
||
HR {border-style: solid; border-color: #000000; background-color:Black;}
works fine
BUT if you do a <hr noshade> it turns to a light to dark grey depending if you
have background or not
Comment 38•22 years ago
|
||
Comment on attachment 25714 [details] [diff] [review]
Patch: removes nsHRFrame and (in standard mode) nsBRFrame, and replaces them with CSS
dbaron: That was the patch I wrote before hyatt changed the style system. It is
obsolete.
Attachment #25714 -
Attachment is obsolete: true
Comment 39•22 years ago
|
||
*** Bug 174423 has been marked as a duplicate of this bug. ***
Comment 40•22 years ago
|
||
*** Bug 177174 has been marked as a duplicate of this bug. ***
Comment 41•22 years ago
|
||
*** Bug 182278 has been marked as a duplicate of this bug. ***
Comment 42•22 years ago
|
||
*** Bug 186716 has been marked as a duplicate of this bug. ***
Comment 43•22 years ago
|
||
*** Bug 187670 has been marked as a duplicate of this bug. ***
Updated•22 years ago
|
Keywords: mozilla1.0
Comment 44•22 years ago
|
||
-moz-bg-inset is treated as a special case at
http://lxr.mozilla.org/seamonkey/source/layout/html/style/src/nsCSSRendering.cpp#489
which invokes at
http://lxr.mozilla.org/seamonkey/source/layout/html/style/src/nsCSSRendering.cpp#199
a rendering based only on the background color. The only place where this
should be needed are tables in quirks mode, because IE and NN render them so
Changing the default border style to inset invokes the usual code that tables
use in standards mode and all other element in any mode. Please note also that
with this change the hr will really look like inset, they look currently like
outset due to bug 14090
Attachment #111282 -
Flags: review?(ian)
Comment 45•22 years ago
|
||
removal of quirks is always fine by me -- r=hixie
However, this shouldn't be considered to fix the bug. For that we need to port
attachment 25714 [details] [diff] [review].
Attachment #111282 -
Flags: superreview?(bzbarsky)
Comment 46•22 years ago
|
||
Comment on attachment 111282 [details] [diff] [review]
patch to allow coloring of <hr>
Please post screenshots of rendering <hr> on the following backgrounds: black,
white, red, green, cyan with this patch and without (and if possible in IE and
NS4)
Comment 47•22 years ago
|
||
Updated•22 years ago
|
Attachment #111282 -
Flags: superreview?(bzbarsky) → superreview+
Attachment #111282 -
Flags: review?(ian)
Comment 48•22 years ago
|
||
*** Bug 193200 has been marked as a duplicate of this bug. ***
Comment 49•21 years ago
|
||
This patch doesn't yet remove the hacky centering code in nsLineLayout, but it
updates the previous patch to the tip and addresses the other issues dbaron
mentioned. It also adds a new test case (for <br> in standard mode). I'll look
at the nsLineLayout stuff now, I just wanted to back this up first.
This patch accidentally fixes bug 69518 in quirks mode. Ironically, in standard
mode it doesn't (even though in standard mode BRs are done using normal CSS
frames). Also, the patch introduces some tabs. I'll clean those out before
asking for a review.
If anybody knows of any testcases for HR or BR, let me know. Cheers.
Comment 50•21 years ago
|
||
How does this affect HR alignment in the presence of floats?
Comment 51•21 years ago
|
||
In theory, it works as before, due to the use of -moz-float-edge: margin-box.
Comment 52•21 years ago
|
||
Untested; I'm updating to tip and building to check these latest changes work.
Anyway, in theory, this removes the remaining mentions of HR frames, e.g. in
nsLineLayout. It also removes a few tabs I came across.
Attachment #125340 -
Attachment is obsolete: true
Comment 53•21 years ago
|
||
Yeah, it compiles fine. I'll run with it for a while to make sure there's no
unexpected regressions.
Updated•21 years ago
|
Attachment #125341 -
Flags: superreview?(dbaron)
Attachment #125341 -
Flags: review?(bz-bugspam)
Comment 54•21 years ago
|
||
Notes:
For size<2, for IE compatability I could nuke the side and bottom borders and
just have the top one. (Trunk doesn't support size<3 at the moment.) This would
fix microsoft.com.
For IE compatability, we could default to size=2 instead of size=3.
Could support color attribute, in IE it just sets the color instead of using
what I've called -moz-bg-solid.
Comment 55•21 years ago
|
||
Comment on attachment 125341 [details] [diff] [review]
Patch: removes all mention of HR frames and (in standard mode) replaced nsBRFrame with CSS
I'm going to do the first and last of those. New patch ccming up.
Attachment #125341 -
Attachment is obsolete: true
Attachment #125341 -
Flags: superreview?(dbaron)
Attachment #125341 -
Flags: review?(bz-bugspam)
Comment 56•21 years ago
|
||
Does that patch fix bug 116909? If so, there might be some lines of
nsBRFrame.cpp that can be removed (see large comment block). If not, is there
an easy way to fix that issue along with this one?
Comment 57•21 years ago
|
||
No, it doesn't solve that bug, and I don't want to roll a fix for that bug into
this one (I'm not sure how to fix it). Incidentally, I just noticed that this
removes the almost standards mode check. I'll make BR frames be used in that
mode too instead of only quirks mode.
Comment 58•21 years ago
|
||
This fixes the issues I mentioned above.
Updated•21 years ago
|
Attachment #125417 -
Flags: superreview?(dbaron)
Attachment #125417 -
Flags: review?(bz-bugspam)
Comment 59•21 years ago
|
||
Comment 60•21 years ago
|
||
From left to right:
IE6/win32 Moz/win32 Moz/linux Opera7/linux NN4.78/linux
The red lines were added by me to help counting the pixels - it shows that we
are consistently drawing 1 pixel more than SIZE is set to, which is different
from other browsers. I guess this explains why it currently looks as if the
default in Mozilla is SIZE=3. Could you attach a screenshot of Testcase #3
with your patch?
Also note the difference to other browsers for SIZE=0 and SIZE=1.
Comment 61•21 years ago
|
||
Ok, more regressions: I need to add hr to the list of things that lose their
bottom margin in cells, and this patch causes newlines to act really oddly in
standard mode textareas. (Editor can't handle elements that introduce generated
content? Why am I not surprised.)
Comment 62•21 years ago
|
||
Actually no, they shouldn't lose their bottom margin. Something else odd is
going on here.
Updated•21 years ago
|
Attachment #125417 -
Flags: superreview?(dbaron)
Attachment #125417 -
Flags: review?(bz-bugspam)
Comment 63•21 years ago
|
||
Attachment #111282 -
Attachment is obsolete: true
Attachment #111302 -
Attachment is obsolete: true
Attachment #125417 -
Attachment is obsolete: true
Comment 64•21 years ago
|
||
(That screenshot was taken after changing font zoom to work around bug 209066.)
Comment 65•21 years ago
|
||
Ok, this patch breaks multiple newlines in <textarea>s and multiple <br>s (due
to bug 209073), so it's not ready for review after all.
Comment 66•21 years ago
|
||
This is just the previous patch, updated to the tip.
I'm going to extract the BR changes and just post a patch with the HR ones,
since they are more mature.
Comment 67•21 years ago
|
||
Comment 68•21 years ago
|
||
Comment on attachment 126235 [details] [diff] [review]
Just the HR part of the patch
Ok, this really should be ready for review. I removed the problematic part (the
<br> stuff). This patch removes the <hr> frame, using just CSS and attribute
mapping for its rendering. It also improves our compatability with IE, allowing
<hr>s to be smaller than before, and adding support for a "color" attribute.
This makes www.microsoft.com look much closer to its rendering in IE.
There are several parts I'm really not sure of, especially the bit where I
implement a new interface. I couldn't find any firm documentation on how to do
that, so this is mainly guesswork based on hints from bbaetz.
Attachment #126235 -
Flags: review?(bzbarsky)
Comment 69•21 years ago
|
||
Comment on attachment 126235 [details] [diff] [review]
Just the HR part of the patch
>Index: content/html/content/src/nsHTMLHRElement.cpp
>+#include "nsIDOMHTMLHRElement2.h"
For consistency with other such compat interfaces, please call that
nsIDOMNSHTMLHRElement
>+ else if (aAttribute == nsHTMLAtoms::color) {
>+ if (aResult.ParseColor(aValue, mDocument)) {
>+ return NS_CONTENT_ATTR_HAS_VALUE;
>+ }
>+ }
That will work incorrectly when setting innerHTML, eg, since mDocument is null
then. You need to get the document from mNodeInfo if mDocument is null (the
other callers of ParseColor() were also doing that incorrectly until recently).
>+ nsHTMLValue value;
>+ bool noshade;
PRBool, please. And PR_TRUE/PR_FALSE later on.
>+ bool allSides = true;
PRBool.
>+ } else
>+ sizePerSide = 1; // default to a 2px high line
Put curly braces around the else body if the if body had them, please. And
1.0f instead of 1 here.
>+ // if a color is set, set the border-style to 'solid' so that the
>+ // 'color' property takes effect, otherwise, use '-moz-bg-solid'.
>+ // (we got the color attribute earlier)
>+ int style;
>+ if ((color.GetUnit() == eHTMLUnit_Color) ||
>+ (color.GetUnit() == eHTMLUnit_ColorName))
We keep doing that check.. could we save that boolean up front somewhere? Also
those tests are overparenthesized.
>+ if (((color.GetUnit() == eHTMLUnit_Color) ||
>+ (color.GetUnit() == eHTMLUnit_ColorName)) &&
>+ (aData->mColorData->mColor.GetUnit() == eCSSUnit_Null)) {
Again, overparenthesized conditionals.
>Index: layout/base/public/nsStyleConsts.h
> #define NS_STYLE_BORDER_STYLE_BG_INSET 11
> #define NS_STYLE_BORDER_STYLE_BG_OUTSET 12
>+#define NS_STYLE_BORDER_STYLE_BG_SOLID 13
It would be good to document what those three do....
>Index: layout/html/base/src/nsLineLayout.cpp
>- // here is where we do special adjustments for HR's
>- // see bug 18754
I assume that you have tested that you are not regressing that?
>Index: layout/html/style/src/nsCSSRendering.cpp
>+ np = MakeSide (theSide, aContext, whichSide, borderOutside, borderInside,aSkipSides,
I know you just copied this, but add a space before aSkipSides.
>+ //aContext.DrawLine (theSide[0].x, theSide[0].y, theSide[1].x, theSide[1].y);
>+ DrawLine (aContext, theSide[0].x, theSide[0].y, theSide[1].x, theSide[1].y, aGap);
Why copy this commented thing?
> Index: dom/public/idl/html/nsIDOMHTMLHRElement.idl
>+ // attribute DOMString color; (see nsIDOMHTMLHRElement2.idl)
Just remove this; don't comment it.
> Index: dom/public/idl/html/nsIDOMHTMLHRElement2.idl
>+ // attribute DOMString align;
>+ // attribute boolean noShade;
>+ // attribute DOMString size;
>+ // attribute DOMString width;
No need for those.
It looks like this patch fixes bug 84887, right? And perhaps bug 119606 and
bug 165261 ?
Attachment #126235 -
Flags: review?(bzbarsky) → review-
Comment 70•21 years ago
|
||
>>Index: layout/base/public/nsStyleConsts.h
>> #define NS_STYLE_BORDER_STYLE_BG_INSET 11
>> #define NS_STYLE_BORDER_STYLE_BG_OUTSET 12
>>+#define NS_STYLE_BORDER_STYLE_BG_SOLID 13
>
> It would be good to document what those three do....
The NS_STYLE_BORDER_STYLE_BG_* constants (-moz-bg-* keywords) are hacks that use
the background colour instead of the foreground colour when they paint. They
should probably be implemented as normal styles with a special colour keyword,
but for historical reasons, they are not.
>>Index: layout/html/base/src/nsLineLayout.cpp
>>- // here is where we do special adjustments for HR's
>>- // see bug 18754
>
> I assume that you have tested that you are not regressing that?
I did test that things worked ok around floats, yes. (This patch uses
-moz-float-edge, which recently started working more reliably.)
> It looks like this patch fixes bug 84887, right? And perhaps bug 119606 and
> bug 165261 ?
This should fix most of the dependencies, I think.
Anyway, for obvious reasons, I won't be able to fix this and check it in. Could
I ask you to do the fixups and checkin? They all seem pretty simple.
Comment 71•21 years ago
|
||
caillon: this is the bug i mentioned
Assignee: waterson → caillon
Status: ASSIGNED → NEW
Comment 72•21 years ago
|
||
*** Bug 213078 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 73•21 years ago
|
||
Latest, greatest. This exposes another layout bug, though. I think bz is
going to file that shortly and post the link here.
Attachment #126235 -
Attachment is obsolete: true
Comment 74•21 years ago
|
||
Comment on attachment 128831 [details] [diff] [review]
Patch
>Index: content/html/content/src/nsHTMLHRElement.cpp
>+ PRBool noshade;
PRBool noshade = PR_FALSE;
please, to shut up the idiotic compiler and the people who will spam this bug
with "added a warning" crap.
>Index: layout/html/style/src/nsCSSRendering.cpp
>+ aRenderingContext.SetColor(MakeBevelColor(aSide, border_Style, bgColor->mBackgroundColor, sideColor,
>+ MOZ_BG_BORDER(border_Style) ? PR_FALSE : PR_TRUE));
Ummm... how about making that last arg be "!MOZ_BG_BORDER(border_Style)" ?
>Index: dom/public/idl/html/nsIDOMNSHTMLHRElement.idl
>+ * The Initial Developer of the Original Code is
>+ * Netscape Communications Corporation.
>+ * Portions created by the Initial Developer are Copyright (C) 2003
>+ * the Initial Developer. All Rights Reserved.
>+ *
>+ * Contributor(s):
>+ * Vidur Apparao <vidur@netscape.com> (original author)
>+ * Johnny Stenback <jst@netscape.com>
All of that is a blatant lie. Please fix to reflect reality (original
developer is hixie, there are no contributors except maybe caillon).
With those nits, r+sr=me. The bug I was seeing was bug 209066
Attachment #128831 -
Flags: superreview+
Attachment #128831 -
Flags: review+
Comment 75•21 years ago
|
||
Since nobody else mentioned it on the bug, this was checked in to the trunk
2003-07-30 01:12/13 -0700.
Comment 76•21 years ago
|
||
From comment #32, the attached example (attachment id 64622) in bug 119606 still
shows issues with using the style attribute/css to color HR's. (See screenshots
for difference.)
Comment 77•21 years ago
|
||
bug 119606 is showing correct behavior, in fact. If you want to style <hr> via
CSS, you need to style the borders, which are what you see. Changing the
"color" property in CSS will not help there.
I'm marking this bug fixed, since it started out as an <hr> bug and the <hr>
part is fixed. Please file follow-ups on the <br> problem, if any, as needed.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•