112990 - Support multiple levels of border using CSS

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[David Hyatt]

This bug covers the ability to specify arbitrary multiple levels of border using
CSS.

Comment 1

[Boris Zbarsky [:bzbarsky]]

mpt was looking for something like this for styling links...

Comment 2

[David Hyatt]

Attachment: Patch to implement multiple border colors for a given border side

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 60418 [details] [diff] [review]
Patch to implement multiple border colors for a given border side

>Index: content/base/src/nsRuleNode.cpp
>+                  if (prop->mayHaveExplicitInherit)
>+                    canHaveExplicitInherit = PR_TRUE;

You can just make this an assertion since the one property that
has this type has PR_FALSE for mayHaveExplicitInherit.

>Index: content/html/style/src/nsCSSDeclaration.cpp
>+  if (aCopy.mBorderColors) {
>+    PRInt32 i;
>+    if (!mBorderColors) {

This is a copy-constructor, so you need to change two
things:
 * initialize mBorderColors to null in the member initializer
   list
 * not null-check it here, since it's guaranteed to be null


Don't you need to patch SetValueImportant as well, just like
AppendValue?

>Index: content/html/style/src/nsCSSParser.cpp

more comments later -- I don't trust this text editor that much. :-(

Comment 4

[David Hyatt]

Ok, I've made those changes.

Comment 5

[David Hyatt]

Attachment: Patch v2, incorporates dbaron's suggestions.

Comment 6

[Joe Hewitt (gone)]

Comment on attachment 60433 [details] [diff] [review]
Patch v2, incorporates dbaron's suggestions.

r=hewitt

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In nsStyleStruct.h:
 * maybe you could be a little less malloc-happy (which linked lists tend to be)
-- is there any chance we could at least allocate arrays?  And maybe even use
the presShell arena?
 * Shouldn't the mBorderColors member of nsStyleBorder be protected?
 * Should some of the new members of nsStyleBorder be non-inline?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Your condensation of the loop in nsCSSRendering::PaintBorder changes the order
the border sides are drawn, which is probably a substantive change we don't want
-- it messes up the corners.  You could probably still condense the code by doing:

static PRUint8 sideOrder[] = { NS_SIDE_BORDER, NS_SIDE_LEFT, NS_SIDE_TOP,
NS_SIDE_RIGHT};

and then:
  PRUint8 side = sideOrder[cnt];

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(And, in my previous comment, you should probably change count to an unsigned
int or PRUint32.)

You might want to comment your hacking painting of the background in the padding
area only and note that it won't work if you support dashed, dotted, or
transparent borders in -moz-border-colors.

You should also comment that you're ignoring |aSkipSides| in DrawCompositeBorder
-- you should probably just drop that parameter and pass 0 to MakeSide (which
uses it to draw an appropriate bevel for a border on a split element (for
printing or for inline elements).  Ignoring aSkipSides does make this feature
pretty bad for HTML use, if it weren't already.  Or you could check |aSkipSides|
properly even though it will never be set for XUL.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

One final comment -- I'll assume you got all the math right in
DrawCompositeBorder, but this should really be tested on multiple toolkits to
make sure DrawLine is consistent across toolkits before we rush ahead and use it
in the themes.

Comment 11

[David Hyatt]

Fixed.

Comment 12

[[e-mail]]

Interesting effect -moz-border-radius has on -moz-border-XXX-colors!