542406 - readonly="true" doesn't work on fields

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Many of the fields in tabbrowser have readonly="true" set - except this doesn't work on fields. That attribute should be removed.

Comment 1

[Dão Gottwald [:dao]]

Per https://developer.mozilla.org/en/XUL_Tutorial/Adding_Properties_to_XBL-defined_Elements#Readonly_Attribute>, it should work, so if it doesn't, I guess that's an XBL bug.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Per the docs you'd expect it to work, yes. Consensus on IRC was that it has never worked, because the XBL1 spec isn't fully implemented. I'd be *nice* if it did work.

Comment 3

[Dão Gottwald [:dao]]

(In reply to comment #2)
> Consensus on IRC was that it has
> never worked, because the XBL1 spec isn't fully implemented.

If there's also consensus that it won't be implemented, the documentation needs to be updated.

Comment 4

[Boris Zbarsky [:bzbarsky]]

So... This has been broken ever since Harish's "fix ports bustage" patch in bug 105137.  He changed the arg type of aReadOnly in nsXBLProtoImplField::nsXBLProtoImplField from |const nsAReadableString*| to |PRUnichar*| but didn't change this code:

62     nsAutoString readOnly; readOnly.Assign(*aReadOnly);
63     if (readOnly.LowerCaseEqualsLiteral("true"))

Of course now that just assigns a single char, so always tests false.

This is trivial to fix (drop the '*' from that Assign call).  How many consumers would it break to do that?  ;)

I really wish people back then bothered to have regression tests.  :(

Comment 5

[Mike Connor [:mconnor]]

I don't think it'll break consumers of XBL... though any extensions that depend on setting readonly fields on bindings wouldn't be happy.  They can deal, IMO.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 423811 [details] [diff] [review]
Fix

Thanks bz!

Comment 8

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/6de23691694a

Comment 9

[Eric Shepherd [:sheppy]]

Updated:

https://developer.mozilla.org/en/XUL/Attribute/readonly
https://developer.mozilla.org/en/XUL_Tutorial/Adding_Properties_to_XBL-defined_Elements#section_6

Comment 10

[Nickolay_Ponomarev]

removing dev-doc-complete: https://developer.mozilla.org/en/XUL/Attribute/readonly is about a XUL attribute, this bug is about XBL <field readonly="true">. The docs now incorrectly state that e.g. https://developer.mozilla.org/en/XUL/preference#a-readonly did not work before 1.9.3.

There's also no way to figure out from https://developer.mozilla.org/en/Upcoming_Firefox_features_for_developers#Changes_for_Mozilla_and_add-on_developers what the fix was about.

(In addition to this, MDC is almost down)

Comment 11

[Florian Scholz (Open Web Docs)]

Changed docs. See bug 62258.

Comment 12

[Florian Scholz (Open Web Docs)]

Sorry, it's bug 622581