46954 - Returned values do not conform to HTML4.01 spec for align attribute[form sub]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[ckritzer (gone)]

Overview:  When returning expected values for HTMLInputElements.align, the two 
values "Bottom" and "Middle" return "Baseline" and "Center", respectively.  
Reference the HTML4.01 spec with regards to align property:
	http://www.w3.org/TR/html4/struct/objects.html#h-13.7.4

Steps to reproduce:
1) Launch build
2) Load URL or attached testcase

Actual Results:  "Baseline" and "Center"

Expected Results: "Bottom" and "Middle"

Builds Found On: 
 - MacOS9 2000-07-25-11-M17
 - Linux6 2000-07-25-08-M17
 - Win98  2000-07-25-09-M17

Additional Info:
n/a

Comment 1

[rods (gone)]

Eric, I think this might be yours, if not send it back.

Comment 2

[ckritzer (gone)]

PDT:  Nominating nsbeta3+
Reason: Correctness, HTML4 spec

Comment 3

[Kevin McCluskey (gone)]

Marking nsbeta3-

Comment 4

[bsharma]

Updating QA contact.

Comment 5

[Boris Zbarsky [:bzbarsky]]

nominating

Comment 6

[Kevin McCluskey (gone)]

Setting milestone to mozila0.9.1

Comment 7

[Vladimir Ermakov]

QA Contact Update

Comment 8

[Kevin McCluskey (gone)]

Bulk reassigning form bugs to Alex

Comment 9

[Madhur Bhatia]

---> DOM HTML?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Looks invalid to me.  An HTMLInputElement is not an object, so the cited part of
the spec does not apply.  From
http://www.w3.org/TR/html4/interact/forms.html#h-17.4 we have a link to
http://www.w3.org/TR/html4/present/graphics.html#adef-align which says:

align = left|center|right|justify [CI] 

Other values of align make no sense for an HTMLInputElement....

Or am I missing something?  The testcase cited no longer exists, by the way.

Comment 11

[Alexandru Savulov]

this might be more serious than it looks like:

in nsGenericHTMLElement.cpp we have:

static nsGenericHTMLElement::EnumTable kAlignTable[] = {
  { "left", NS_STYLE_TEXT_ALIGN_LEFT },
  { "right", NS_STYLE_TEXT_ALIGN_RIGHT },

  { "texttop", NS_STYLE_VERTICAL_ALIGN_TEXT_TOP },// verified
  { "baseline", NS_STYLE_VERTICAL_ALIGN_BASELINE },// verified
  { "center", NS_STYLE_VERTICAL_ALIGN_MIDDLE },
  { "bottom", NS_STYLE_VERTICAL_ALIGN_BASELINE },//verified
  { "top", NS_STYLE_VERTICAL_ALIGN_TOP },//verified
  { "middle", NS_STYLE_VERTICAL_ALIGN_MIDDLE },//verified
  { "absbottom", NS_STYLE_VERTICAL_ALIGN_BOTTOM },//verified
  { "abscenter", NS_STYLE_VERTICAL_ALIGN_MIDDLE },/* XXX not the same as ebina */
  { "absmiddle", NS_STYLE_VERTICAL_ALIGN_MIDDLE },/* XXX ditto */
  { 0 }
};

If you take a look, "center" and "middle" map to the same value:

NS_STYLE_VERTICAL_ALIGN_MIDDLE

and "baseline" and "bottom"  to:

NS_STYLE_VERTICAL_ALIGN_BASELINE

Now the forwards mapping will work be cause we map from different values to the
same value.

The reverse mapping will work incorrect if the map is walked forward (starting
with the 0th element) for the values "bottom", "absbottom", "middle",
"absmiddle", "abscenter" and this will happen on every element that uses the
attribute "align", not only the <input> element.

A solution would be the creation of new macros for each particular value, but
this requires the search for all the places where the macros are used. 

Another solution would be to add an aditional member keeps this information to
each HTMLElement that uses align (but this is very un-cool)

Changing the component: DOM HTML.

Comment 12

[Alexandru Savulov]

ok there are:

12 occurrences of NS_STYLE_VERTICAL_ALIGN_BASELINE

and

13 occurrences of NS_STYLE_VERTICAL_ALIGN_MIDDLE

in the source. I think that the sollution of creating new macros is the one
that's more valuable.

Comment 13

[Alexandru Savulov]

Attachment: proposed fix

Comment 14

[Fabian Guisset]

why do we have baseline and center in the first place? for netscape 4 compat?
the patch can't harm though...

Comment 15

[Fabian Guisset]

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

Comment 16

[Johnny Stenback (:jst)]

Not sure what this does to the stylesystem, will NS_STYLE_VERTICAL_ALIGN_CENTER
really map to the correct display with this change? Or does it not matter?

Someone who knows the stylesystem should review, assuming this is ok from a
stylesystem point of view, sr=jst

Comment 17

[Alexandru Savulov]

ok... will ask Attinasi for a review from the stylesystem point of view

Comment 18

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

We shouldn't have two style consts that mean the same thing -- that 
makes the code more confusing.  Isn't the right solution to have a 
separate attribute table?

Comment 19

[Alexandru Savulov]

Attachment: testing 4 different ways of using align="..."

Comment 20

[Alexandru Savulov]

dbaron:
Are you proposing to separate the values in a legal and an illegal mapping table,
then chose which table to use? Aren't we then constrained to keep information
around that leads us back to the proper table when we reverse-map?
Could you please explain your ideea?

Comment 21

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

No, only that you use a different table for the elements with each different 
set of legal values for the align attribute (which should vary by element).  I 
think we already do that to some extent, but it may not quite be done 
correctly.

Comment 22

[Alexandru Savulov]

Well, the problem is that we have to deal with legal and illegal values for a
gorup of elements that share the same set of legal values. (in our case the
elements are APPLET, IFRAME, IMG, INPUT, OBJECT and the legal values for them
are "left", "right", "top", "middle", "bottom".
Now if some web-dev is using "center" instead of "middle" we want to handle the
illegal case like we would handle the legal attribute and still keep track of
the original value. (another solution than the proposed patch is to use the same
constant for both but then we have to buy the loss of the initial values and we
have to keep the order in the table - legal perecedes illegal - but this is also
risky cause someone can change that order)

Comment 23

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

Legal precedes illegal is the way we have done it for all other attributes and I
see no reason why we should make the style code more confusing by changing that.
 If there are elements with different sets of legal values but the same set of
legal+illegal values, then they should use different mapping tables.

Comment 24

[Mats Palmgren (inactive)]

This was fixed by bug 192077.

-> FIXED