User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; ja; rv:1.9.2pre) Gecko/20100110 Namoroka/3.6pre (ayakawa PGU) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; ja; rv:1.9.3a1pre) Gecko/20100114 Minefield/3.7a1pre logon cocolog(above url), and show editor view. in this view, toolbar is made table. on Fx 3.6RC, edit toolbar has no border. But, on trunk, edit toolbar has border. Reproducible: Always Steps to Reproduce: 1.logon cocolog 2.show editor view 3. Actual Results: toolbar is drawing with border. Expected Results: toolbar is drawing without border. OK : changeset 36971:6a7294d0f305 NG : changeset 36974:2ac64027f304

http://www.w3.org/TR/html4/struct/tables.html#adef-border-TABLE defines border as pixel the both failing test cases use 0px and 0em which I did not expect

fantasai do think there could be a selector that would match those cases too.

s/do/do you/

Not that I can think of. We don't have regular expression matching in selectors, and we'd need something like [border="\d+"] to prevent matches against invalid values. This probably needs to be handled in the C++ rule mapping. We could set the width to zero where we set the width for border attribute values.

I think we need to block on this; it's a regression since 3.6.

Do we know: * how our current behavior differs from what we implemented before? * whether other browsers match what we implemented before? Those would be helpful in figuring out what needs to be fixed here.

https://bugzilla.mozilla.org/show_bug.cgi?id=577654 describes pretty good the problem

the fx3.6 rendering is identical since gecko 1.8.1

It gets worse - when specified as 'something em', the border is 1 pixel wide regardless.

(In reply to comment #9) > Do we know: > * how our current behavior differs from what we implemented before? Trunk now has a border when border="0px", whereas 1.9.2 and earlier had no border. The 1 pixel border when border="#em" occurs at least as early as 1.9.0. > * whether other browsers match what we implemented before? Webkit, IE and Opera all do as expected (just use the number part of the attribute).

I think the most straightforward way to fix this is to implement a :-moz-borderzero selector in the style system that does what we want. (If we need this for other things we could extend it to get the numeric part of attribute values at some point in the future.) I can do that, though probably not this week.

(In reply to comment #12) > Created attachment 469275 [details] > testcase with different border widths Is it ok with you if I check this testcase in as a reftest?

This reverts things to the 1.9.2 behavior.

Comment on attachment 477791 [details] [diff] [review] patch Jonas also gets a chance to yell at me for doing this, and maybe even suggest something nicer.

Comment on attachment 477791 [details] [diff] [review] patch >+++ b/layout/style/nsCSSRuleProcessor.cpp >+ const nsAttrValue *val = >+ ge->GetAttrInfo(kNameSpaceID_None, nsGkAtoms::border).mValue; nsAttrValue* val = ge->GetParsedAttr(nsGkAtoms::border); r=me with that.

(In reply to comment #15) > (In reply to comment #12) > > Created attachment 469275 [details] [details] > > testcase with different border widths > > Is it ok with you if I check this testcase in as a reftest? Sure. (In reply to comment #16) > Created attachment 477791 [details] [diff] [review] > patch > > This reverts things to the 1.9.2 behavior. Are the "em"s going to be fixed at some point?

(In reply to comment #19) > (In reply to comment #16) > > Created attachment 477791 [details] [diff] [review] [details] > > patch > > > > This reverts things to the 1.9.2 behavior. > > Are the "em"s going to be fixed at some point? Probably that should be a separate bug; this basically gets things in sync again by using our existing HTML integer attribute parsing code rather than only checking for "0"; that would be a change to our integer attribute parsing code. If there isn't a bug on file already, it's worth filing.

It seems like it's probably covered by bug 175788, which actually might be easier to fix now that HTML5 specifies what we should do.

So, actually, despite what the comment in the table[frame] rule says, it turns out this breaks layout/reftests/table-bordercollapse/borderhandling-frame-border.html . So I think I should reorder it to match what it used to do.

Actually, we've already changed our handling of the frame attribute without a value for this release, I think intentionally, so I think the right thing to do to be consistent is to fix the test.

Comment on attachment 477791 [details] [diff] [review] patch The only thing that is weird here is that the selector will be somewhat arbitrary as to which elements it "works" on, since many elements doesn't parse border attributes into an integer. The implementation isn't "table" specific in any way either, though it might be that it's mostly table elements that parse border attributes? So please add a comment to that extent somewhere (do we document internal pseudo classes anywhere?) r=me

Er... I *meant* to make it table-specific, but I forgot.

I meant to check the tag as well.

Comment on attachment 479091 [details] [diff] [review] patch r=me

Er, I just realized that patch doesn't handle dynamic changes. Can you fix that, please?

Indeed. I meant to add the code for that. There isn't any problem changing these from checking atoms to checking the pseudoclass enums, is there? It ought to be faster. (Why do we still keep the atom around?) I also added a test, though it passes without the change, presumably because of the handling in nsHTMLStyleSheet/nsHTMLTableElement (for the border of the table itself).

Comment on attachment 481247 [details] [diff] [review] patch r=me. As far as getting rid of the atoms, I'm not sure why we haven't yet. I seem to recall there being something that depended on them... but I can't remember what. File followups to work on eliminating them?

Filed bug 602341.