52531 - Setting and removing table cell border produces errors (removeproperty shorthand)

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Taras Tielkes]

The attached testcase has a minimal HTML table with one table cell.

A click on the first button will attempt to set the border of the table cell to
"5px solid red". This happens, but now the cell contents is rendered on top of
the border.

A click on the second button will attempt to remove the border by setting the
style.border attribute to "". The border doesn't disappear, but some rendering
change can be seen.

Comment 1

[Taras Tielkes]

Attachment: Testcase

Comment 2

[Taras Tielkes]

Note that clicking "Change", "Delete", "Change" will not produce the same table
border as clicking "Change" one time. This is also unexpected.

Comment 3

[Taras Tielkes]

Attachment: Modified testcase

Comment 4

[Taras Tielkes]

Attachment: Modified testcase

Comment 5

[Taras Tielkes]

(Accidentaly uploaded the second testcase twice).

The second testcase also sets a separate borderRight style. This difference in 
observed behaviour appears to be: when clicking the second button to remove the 
border, a small rendering difference (pixel shift) is no longer observed.

The second testcase also appears to draw incorrect borders. The right border 
and the others no longer connect.

Comment 6

[Keyser Sose]

Still seeing this in Mozilla Build 2001012205 Win32.

Comment 7

[Amarendra Hanumanula]

QA contact update

Comment 8

[Taras Tielkes]

Still seeing this in a 4/4/20001 build. The rendering error is pretty visible in
both testcases.

Comment 9

[karnaze (gone)]

Moving to m1.0.

Comment 10

[karnaze (gone)]

Attachment: patch to fix the bug

Comment 11

[karnaze (gone)]

In the testcase, I was getting an assertion in style code when deleting the 
border until I changed td.style.border = "" to td.style.border = "0px". Maybe 
the former syntax is incorrect.

Comment 12

[Asa Dotzler [:asa]]

0.9.5 is out the door. bumping TM up by one.

Comment 13

[karnaze (gone)]

*** Bug 100829 has been marked as a duplicate of this bug. ***

Comment 14

[karnaze (gone)]

The patch is in.

Comment 15

[Pierre Saslawsky]

*** Bug 43745 has been marked as a duplicate of this bug. ***

Comment 16

[Amarendra Hanumanula]

 Chris I still see the problem on 2002012003 build Platform WIN2k. The border 
does not get deleted when I hit the second button " delete border". Is this a 
regression?
I am reopening this bug.

Comment 17

[Matthew Mastracci]

Neither testcase works for me either- 2002020703 Win2000

Comment 18

[karnaze (gone)]

Comment #11 still holds. ->style system.

nsDebug::Assertion(const char * 0x02821144, const char * 0x00596264, const char 
* 0x0282110c, int 4782) line 280 + 13 bytes
nsDebug::Error(const char * 0x02821144, const char * 0x0282110c, int 4782) line 
463 + 22 bytes
nsCSSDeclaration::GetValue(nsCSSProperty eCSSProperty_border, nsCSSValue & 
{...}) line 4782 + 21 bytes
nsCSSDeclaration::RemoveProperty(nsCSSProperty eCSSProperty_border, nsCSSValue & 
{...}) line 4037 + 16 bytes
nsDOMCSSAttributeDeclaration::RemoveProperty(nsDOMCSSAttributeDeclaration * 
const 0x02ff4930, const nsAString & {...}, nsAString & {...}) line 94 + 16 bytes
CallSetProperty(nsDOMCSSDeclaration * 0x02ff4930, const nsAString & {...}, const 
nsAString & {...}) line 220 + 23 bytes
nsDOMCSSDeclaration::SetBorder(nsDOMCSSDeclaration * const 0x02ff4934, const 
nsAString & {...}) line 247 + 39 bytes
XPTC_InvokeByIndex(nsISupports * 0x02ff4934, unsigned int 18, unsigned int 1, 
nsXPTCVariant * 0x0012d270) line 106
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode 
CALL_SETTER) line 1994 + 42 bytes
XPCWrappedNative::SetAttribute(XPCCallContext & {...}) line 1841 + 14 bytes
XPC_WN_GetterSetter(JSContext * 0x00b12830, JSObject * 0x01a046d8, unsigned int 
1, long * 0x01adcf68, long * 0x0012d560) line 1290 + 12 bytes
js_Invoke(JSContext * 0x00b12830, unsigned int 1, unsigned int 2) line 788 + 23 
bytes
js_InternalInvoke(JSContext * 0x00b12830, JSObject * 0x01a046d8, long 28213560, 
unsigned int 0, unsigned int 1, long * 0x0012e3cc, long * 0x0012e3cc) line 880 + 
20 bytes

Comment 19

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

Mmm.  RemoveProperty doesn't work for shorthands because GetValue doesn't work
for shorthands.  Then again, what exactly should setting a shorthand to the
empty-strict do?

Comment 20

[Greg K.]

Reconfirmed using FizzillaCFM/2002070913. Setting All/All.

Comment 21

[Boris Zbarsky [:bzbarsky]]

I would say that setting a shorthand to empty should set all the sub-props to empty.

What this means is that either nsCSSDeclaration::RemoveProperty() on a shorthand
should be smart or the DOM code should be smart.

We really need a way to associate a shorthand with the list of props is sets so
that we can do the little "for all properties associated with this shorthand, do
this" dance.  I would really like to stick this info into nsCSSPropList.h somehow...

Comment 22

[Hixie (not reading bugmail)]

per csswg, setting a removing a shorthand should remove all related properties.

Comment 23

[Marco Perez [:mpk]]

Attachment: simple border property test suite

This simple test suite demonstrates how setting some border style properties
affects the other ones. It might be useful for determining and sorting out the
behaviour for all (or most ;-) possible cases. This testcase won't work on IE.
If someone is interested in testing and comparing Mozilla with other browsers,
I'll modify the test suite to work on other browsers as well.

Comment 24

[Boris Zbarsky [:bzbarsky]]

This was fixed by the checkin for bug 125246