This project is archived and is in readonly mode.

#704 ✓duplicate
Neil Jenkins

setProperty('type') fails on <button> elements in Safari 4.0 on mootools 1.2.3

Reported by Neil Jenkins | July 6th, 2009 @ 11:04 AM | in 1.3.0 rc2 (closed)

In Safari (tested on v4.0.1 and v3.0.2), the type property on elements can only be set using button.setAttribute('type', value). Setting button.type = value fails silently - it seems to be read only. The change introduced by checkin 2d41e5655f681f7cb0e2e05fa2ea2ebb75ed706d, which aimed at fixing ticket #669, added 'type' to the list of attributes which use the direct property of the element object rather than the setAttribute/getAttribute methods (the first release with this change is v1.2.3). This is good for getting the property as button.type always returns the type (at least in in Safari 4, Firefox etc. but not in Safari 3), whilst button.getAttribute('type') only returns it once it has been set with setAttribute. However, to allow the property to be set on Safari we must use the setAttribute method when setting this value.

Recommended fix:

if (Browser.Engine.webkit) Element.Properties.type = {

set: function(attribute, value) {
    (value == undefined) ? this.removeAttribute(attribute) : this.setAttribute(attribute, '' + value);
    return this;
}

}

The if statement is optional. It will work just as well in other browsers and may be slightly quicker than the default setProperty since it does not have to check the attributes hash then then the bools hash etc.

Attached is a test case to verify the behaviour of Safari.

Comments and changes to this ticket

  • Neil Jenkins

    Neil Jenkins July 6th, 2009 @ 11:46 AM

    Sorry, that fix was incorrect. Should have been:

    if (Browser.Engine.webkit) Element.Properties.type = {
        set: function(value) {
            (value == undefined) ? this.removeAttribute('type') : this.setAttribute('type', '' + value);
        }
    };
    

    Extra note: this patch only fixes the set function, not the setProperty one. Probably better to rewrite the setProperty function.

  • Fábio M. Costa

    Fábio M. Costa July 6th, 2009 @ 01:22 PM

    It fails on IE too, and theres no patch for it.
    http://webbugtrack.blogspot.com/2007/09/bug-237-type-is-readonly-at...

    It works on other browsers so this might be a bug on safari 4?
    Could you test it on safari 3 so we can see if its a bug on safari 4 or in mootools? (i dont have safari 3 here).

    Thanks!

  • Fábio M. Costa

    Fábio M. Costa July 6th, 2009 @ 01:29 PM

    Oh, sorry i see now that you tested on safari 3.
    If i were you i would use setAttribute on your code since it is available.
    set/get attributes is kind of tricky... since el.type works fine for getting, el.type = 'some' should be fine for setting...

    This will be considered for 2.0.

  • Neil Jenkins

    Neil Jenkins July 6th, 2009 @ 02:55 PM

    The IE bug you are referring to is not actually quite the same. The point with IE is that you cannot change the type attribute of an element after it has been added to the DOM (I believe they view this as a security measure, although it doesn't actually make anything more secure). This is independent of whether you try to assign to the type attribute or use the setAttribute method. The bug in Safari applies to <button> elements and applies irrespective of whether the element has been added to the DOM or not; it is simply that the type attribute is read-only and setAttribute must be used instead. Using setAttribute in my code is inconvenient as it prevents me doing things like:

    var b = new Element('button', {
        type: 'button'
    });
    

    But with the patch from my previous comment this code works as it calls the set function.

  • Fábio M. Costa

    Fábio M. Costa July 6th, 2009 @ 04:34 PM

    I see your point Neil. This fix, introduced in 1.2.3, was to fix the get on 'select-multiple' inputs (getAttribute('type'), returns null on select-multiple elements) and it kind of broke the set on 'button' elements on safari.
    Thanks for the report, again.

  • Scott Kyle

    Scott Kyle September 4th, 2009 @ 03:40 AM

    • Assigned user set to “Scott Kyle”
    • State changed from “new” to “open”
  • Fábio M. Costa

    Fábio M. Costa April 1st, 2010 @ 06:07 PM

    • State changed from “open” to “duplicate”
    • Milestone changed from 2.0 to 1.3.0 rc2

    Ok I'll set this as duplicate as this is pointed out at #747. This is planed to be in 1.3.

  • Christoph Pojer

    Christoph Pojer September 3rd, 2010 @ 01:12 PM

    • Tag changed from regression, setproperty, type to setproperty, type
    • Milestone order changed from “0” to “0”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Shared Ticket Bins

Attachments

Referenced by

Pages