191127 - Applying Content Alignment: Vertical to multiple cells produces broken HTML

Status: VERIFIED FIXED

(SeaMonkey :: Composer, defect)

Description

[Jani Patokallio]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20021130
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20021130

In Table Cell Properties, if Content Alignment: Vertical is adjusted while
multiple cells are selected, nothing seems to happen when OK/Apply is selected,
and the resulting HTML is broken.  Adjustments to Content Alignment: Horizontal
work as they should, and so does changing a single cell's vertical alignment.


Reproducible: Always

Steps to Reproduce:
1. Create a table of at least two cells.
2. Enter content of unbalanced height, like this:
+-----+-----+
| Foo | Bar |
|     | Baz |
|     | Bat |
+-----+-----+
3. Select both cells with click-drag (or method of choice).
4. Attempt to alter Content Alignment: Vertical from default Top to Middle.

Actual Results:  
+-----+-----+
| Foo | Bar |
|     | Baz |
|     | Bat |
+-----+-----+

<td style="vertical-align: top;" valign="middle">Foo<br></td>

...which obviously makes no sense.  Mozilla goes with the first parameter and
valigns top, other browsers may do something different.


Expected Results:  
+-----+-----+
|     | Bar |
| Foo | Baz |
|     | Bat |
+-----+-----+

<td style="vertical-align: middle;">Foo<br></td>

...as correctly produced when selecting and modifying a single cell!


Similar breakage occurs when attempting to modify the vertical alignment of
cells that are already set to something other than "top".

Comment 1

[Kathleen :Brade]

I'd guess if css editing is disabled that the valign attribute is less confused
I'd guess that if this is broken that it is specific to Composer's JS in that
dialog (but that's just my guess).

Comment 2

[[e-mail]]

Attachment: Proposed patch

Comment 3

[[e-mail]]

The problem was that all multiple changes use attributes, instead of respecting
the use CSS style preference.

Comment 4

[Kathleen :Brade]

Comment on attachment 113728 [details] [diff] [review]
Proposed patch

Are there other places where we are calling set/remove Attribute when we should
be calling the "OrEquivalent" versions?

Comment 5

[Jani Patokallio]

Just a layman's comment, but bugs <A
href="http://bugzilla.mozilla.org/show_bug.cgi?id=192217">192217</A> and
<A href="http://bugzilla.mozilla.org/show_bug.cgi?id=128339">128339</A> seem
related.

Comment 6

[[e-mail]]

Jani, typing "bug 192217" and "bug 128339" is better because bugzilla
automagically linkifies them with titles.

Comment 7

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

Comment on attachment 113728 [details] [diff] [review]
Proposed patch

The last parameter or the two *OrEquivalent() calls should be false, meaning
you WANT a transaction for undo/redo.
See definition in
http://lxr.mozilla.org/seamonkey/source/editor/libeditor/html/nsHTMLEditor.h#40
6

Comment 8

[[e-mail]]

Attachment: Fixed patch

I didn't have any docs at the time I wrote the previous patch :-/

Comment 9

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

Comment on attachment 113792 [details] [diff] [review]
Fixed patch

r=glazman

Comment 10

[kinmoz]

Comment on attachment 113792 [details] [diff] [review]
Fixed patch

sr=[e-mail]

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 113792 [details] [diff] [review]
Fixed patch

a=asa (on behalf of drivers) for checkin to 1.3 final.

Comment 12

[Rafael Ebron (:rebron)]

Composer triage team: nsbeta1+/adt2

Comment 13

[[e-mail]]

Fix checked in.

Comment 14

[sairuh (rarely reading bugmail)]

vrfy'd fixed on linux rh8.0 with 2003.02.19.