Bug 3360 – align="right" on a text input aligns the input field to the right

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Thomas Deniau]

I'm not sure if this is a bug, or if the other browsers are wrong, but IE and Firefox render the above URL 
differently.

Safari right-aligns the whole text field.
Firefox right-aligns the text inside the text field, which is the behavior expected by the creators of the 
page I reduced this bug from (in which these two fields let you enter a decimal number - so the two fields 
are in fact reversed in Safari).
IE (Win 6 ; Mac 5) does not align anything.

[Niels Leenheer (HTML5test)]

Created attachment 2155 [details]
Testcase of alignment problem

[Niels Leenheer (HTML5test)]

I discussed this problem with Tantek on #webkit:

rakaz:
  "Tantek: It might make only sense on an image, it is not formally 
   restricted to type=image."

Tantek:	
  "it's clear that <input type="image"> is a replaced object like 
   applet,iframe,img,object"
  "whereas the other inputs are not"
  "and yes, the spec should have been more clear about that"
  "so it is not well defined what align should do on non-image inputs"
  "IMHO the intent of the HTML4 authors was that 'align' only apply to 
   <input type="image">"
  "rakaz, therefore, "align" should have no effect on other inputs"
  "just like an attribute like 'inputAlign="right"' would have no effect"

The behavoir described above by Tantek is consistant with IE 4, 5, 5.5
and 6.0. Also consistant with Opera 7 and 8. It would appear the current
behavoir of both Firefox and Safari is not correct.

[Niels Leenheer (HTML5test)]

Created attachment 2170 [details]
Only let align have any effect when type=image

This patch should only let the 'align' attribute affect an input if the type of
the input is 'image'. Patch is untested.

[Niels Leenheer (HTML5test)]

Created attachment 2180 [details]
Only let align have any effect when type=image (fixed)

Darin Adler wrote:
> What's the purpose of the m_render check? It doesn't make sense to me
> to ignore the ALIGN attribute if it's changed at a time the element
> isn't attached to the render tree. I think we want a patch without it.

m_render check removed.

[Dave Hyatt]

Some comments on this patch:

parseMappedAttribute is called in order on attributes on an element.  You cannot assume that you even 
know the correct type of the element yet (e.g., if the type=image comes *after* the align attribute then 
this patch will still honor the align).  That makes a correct fix for this bug somewhat tricky.

You also need to remove the DOCTYPE and make sure that there was no quirk at play here.  It would be a 
good idea to indicate for Firefox, WinIE and Safari what the current behavior is for both quirks mode and 
strict mode.

[Niels Leenheer (HTML5test)]

> parseMappedAttribute is called in order on attributes on an element.  
> You cannot assume that you even know the correct type of the element 
> yet (e.g., if the type=image comes *after* the align attribute then 
> this patch will still honor the align).  That makes a correct fix for 
> this bug somewhat tricky.

Right, I should have realized this.
How would you feel about the following:

1) Inside parseMappedAttribute you simply break out of the case
   if the id is ATTR_ALIGN. This prevents that the align attribute
   is picked up downstream by HTMLElementImpl::parseMappedAttribute().

2) Move the addHTMLAlignment to HTMLInputElementImpl::attach()

        if ( m_type == IMAGE )
            addHTMLAlignment(getAttribute(ATTR_ALIGN));

   This should prevent the attribute order from having any effect on
   the m_type check.
   

> You also need to remove the DOCTYPE and make sure that there was no 
> quirk at play here.  It would be a good idea to indicate for Firefox, 
> WinIE and Safari what the current behavior is for both quirks mode 
> and strict mode.

Quirks mode is not a factor in this bug. The behavoir is consistant 
whether or not quirks mode is active or not.

[Niels Leenheer (HTML5test)]

Created attachment 2203 [details]
Moved check to ::attach

[Niels Leenheer (HTML5test)]

Created attachment 2209 [details]
expanded patch to include other form elements

In this patch I've also added breaks for the ALIGN attribute for 
the BUTTON and TEXTAREA elements. This prevents them from picking
up a text-align css property downstream from HTMLElementImpl::
parseMappedAttribute.

Additionally, the SELECT element now picks up the ALIGN attribute
and handles it just like and replaced object like IMG and INPUT
TYPE=IMAGE. This behavoir is not defined in any standard, but it
mimics the behavoir of IE/Win 6, IE/Win 5.5, IE/Win 5.0 and 
IE/Win 4.0. Unfortunately, neither Opera nor Firefox exhibits 
this behavoir, so with this patch, Safari will be the only other
browser that mimicks IE's behavoir.

In summary, the following elements are not affected in anyway:
- INPUT (with the exception of TYPE=IMAGE)
- TEXTAREA
- BUTTON

The following elements are affected like a replaced object:
- INPUT (limited to TYPE=IMAGE)
- SELECT

None of the objects mentioned above now pick up a text-align
css style.

[Niels Leenheer (HTML5test)]

An overview of the current behavoir of various other browsers can be 
found here: http://www.rakaz.nl/projects/safari/form-results.html

[Maciej Stachowiak]

Will the latest patch deal with dynamic changes to the value of type or align? Looks to me like doing it in 
attach() will not take care of this. Could you please fix that and also provide a dynamic test case?

[Niels Leenheer (HTML5test)]

Created attachment 2315 [details]
Add support from dynamicly adding an align attribute

Handling dynamic updates to the type attribute should not be
necesarry. Webcore does not allow any updates to the the type after
the first time attach() is called (if I'm correct, this is prevented
by the m_haveType flag). The first call to addHTMLAlignment() happens
from inside the attach() function and after the type is set.

Removing the mapped attribute is something that should be fixed, but
IMHO it should not be fixed in the scope of this bug. It is a larger
problem that also affects images, objects and iframes. Perhaps a
seperate bug should be opened for this.

For example, see the following test case for images. The image element
has an align attribute, this is mapped to two css styles
(vertical-align and float). Dynamically removing the attribute does
not remove the mapped css styles:
http://www.rakaz.nl/projects/safari/images.html

[Niels Leenheer (HTML5test)]

Created attachment 2316 [details]
Testcase of alignment problem (added dynamic test)

Testcase is also available online at:
http://www.rakaz.nl/projects/safari/input-align2.html

[Joost de Valk (AlthA)]

Comment on attachment 2315 [details]
Add support from dynamicly adding an align attribute

This is an orphan, could someone check if this patch would still work?

[David Kilzer (:ddkilzer)]

Bug 7075 is related to this bug.  Bug 7075 was filed because using "align=right" on a textarea in Safari causes the text within the textarea to be right-justified.  Both MSIE 6 and Firefox 1.5.0.1 do NOT right-justify the text.

[Maciej Stachowiak]

Comment on attachment 2315 [details]
Add support from dynamicly adding an align attribute

Looks like this correctly addresses all the previous comments except for the removal issue, which seems like it is a broader problem than just this attribute. So r=me. But I don't think this patch will apply as-is because html_formimpl.cpp has been split since it was made.

[Maciej Stachowiak]

This patch also predates the AtomicString change. It needs to be redone on the TOT code base. This shouldn't be hard but it is not purely mechanicl, so r- for now. I hope someone reworks it soon.

[Sam Weinig]

Created attachment 12146 [details]
patch

This is an updated patch that only deals with the <input> tag, but should handle all of the dynamic cases well.  Included are 2 fleshed out testcases.  I will file a follow up bug to deal with the other form elements.

[Darin Adler]

Comment on attachment 12146 [details]
patch

r=me

[Sam Weinig]

Landed in r18523.