230165 - Can't reset table cell size and change table cell style

Status: RESOLVED FIXED

(SeaMonkey :: Composer, defect)

Description

[Mark Lassau]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031208
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031208

If you add an inline style property using the Table/Cell Properties dialog,
there is no way to remove that style.

Reproducible: Always

Steps to Reproduce:
1. Choose Insert | Table from Table Menu and accept defaults
2. Select a cell and choose "Table Properties" from the Table Menu
3. Choose the "Cells" tab
4. Set the height to 100 pixels and click OK
5. Choose Table | "Table Properties" and the Cell tab again.
6. Clear the "Height" textField (ensure checkbox is enabled)
7. Click OK

Actual Results:  
The "height: 100px;" remains in the style property of the <td> tag.
<td style="vertical-align: top; height: 100px;">

Expected Results:  
Removed the "height: 100px;" part of the inline style.
<td style="vertical-align: top;">

I have "Use CSS styles instead of HTML elements and attributes" checked in
Preferences/Composer (this is the default setting).

Specific information for other parts of this dialog:
Content Alignment:
  There should be a blank option in the drop downs which removes settings.
  Also I believe that the vertical-align should be blank by default (especially
for users with higher-level CSS settings.

Cell Style:
  This component doesn't work. See bug 230160

Text Wrap:
  Choosing "Wrap" actually clears the "white-space: nowrap;".
  This will fail to give expected results if you thought you would override a
"white-space: nowrap;" defined in a higher-level CSS definition.
  It would be nice if this drop-down had <blank>,normal,nowrap,pre.
  This would cover the 3 CSS white-space values.
  The equivalent HTML attribute only has wrap/nowrap so this may be tricky?

Background Color:
  This can be reset successfully.


This issue may have to take into account a trade-off between support for CSS
versus support for deprecated HTML attributes. IMHO we should give CSS
preference as the future of HTML.


Note that in the example, if you use Apply instead of OK, it appears to work,
but it is not really clearing the "Height", it is resetting the last value.
Confirm this by setting 100 pixels, Apply; 200 pixels, Apply; clear the
textfield, Apply.
The Height is reset to 100 pixels.

Maybe this should be broken into separate issues?
I consider the "Size" behaviour to be a bug, Text-wrap and Content-Alignment to
be enhancement requests.

Comment 1

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

confirmed, taking.

Comment 2

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Fix pending.

Comment 3

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Reporter: yes, please slice this bug into pieces. I consider the current one
to be only about sizes and cell style.

Comment 4

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

the bug about being unable to switch a cell to header type is a regression
introduced by brade 2003-jul-28 with fix for bug 209548.

Before patch : 

  // Set to the opposite of current type
  nsAutoString tagName;
  GetTagString(aSourceCell, tagName);
  NS_NAMED_LITERAL_STRING(tdType, "td");
  NS_NAMED_LITERAL_STRING(thType, "th");
  nsString newCellType( (tagName == tdType) ? thType : tdType );

After patch :

  // Set to the opposite of current type                                        
  nsCOMPtr<nsIAtom> atom = nsEditor::GetTag(aSourceCell);                       
  nsString newCellType( (atom == nsEditProperty::th) ? NS_LITERAL_STRING("th") :
 NS_LITERAL_STRING("td"));

and that last piece of code is false.... Should be

  nsString newCellType( (atom == nsEditProperty::td) ? NS_LITERAL_STRING("th") :
 NS_LITERAL_STRING("td"));

Comment 5

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: patch #1

Comment 6

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 138690 [details] [diff] [review]
patch #1

I did not even notice bz had a -vacation suffix...
dbaron, can you sr please ? Thanks.

Comment 8

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

checked in (trunk)

Comment 9

[Scott MacGregor]

I love it when a bug gets fixed just before I start debugging it!

Comment 10

[Mark Lassau]

re: Comment #3
If you haved fixed the Cell Style problem in this bug, then bug 230160 should be
marked as a duplicate of this bug.

Comment 11

[Scott MacGregor]

fixed on the thunderbird m4 branch.

Comment 12

[Boris Zbarsky [:bzbarsky]]

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

Comment 13

[Mark Lassau]

re: Comment #3
Bug 230653 added for Content Alignment.
Bug 230654 added for Text Wrap.

I have changed my mind on the "Enhancement" status of these.
They both can produce "Unexpected results".

Comment 14

[Scott MacGregor]

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

Comment 15

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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

Comment 16

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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