readonly="true" doesn't work on fields

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Core
XBL
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: Unfocused, Assigned: bz)

Tracking

({dev-doc-complete})

Trunk
mozilla1.9.3a1
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Many of the fields in tabbrowser have readonly="true" set - except this doesn't work on fields. That attribute should be removed.
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.
Component: Tabbed Browser → XBL
Product: Firefox → Core
QA Contact: tabbed.browser → xbl
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.
(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.
Keywords: dev-doc-needed
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.  :(
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.
Summary: tabbrowser has readonly="true" on fields, which as no effect → readonly="true" doesn't work on fields
Created attachment 423811 [details] [diff] [review]
Fix
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #423811 - Flags: review?(jonas)
Comment on attachment 423811 [details] [diff] [review]
Fix

Thanks bz!
Attachment #423811 - Flags: review?(jonas) → review+
Pushed http://hg.mozilla.org/mozilla-central/rev/6de23691694a
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED

Updated

7 years ago
Target Milestone: --- → mozilla1.9.3a1
Updated:

https://developer.mozilla.org/en/XUL/Attribute/readonly
https://developer.mozilla.org/en/XUL_Tutorial/Adding_Properties_to_XBL-defined_Elements#section_6
Keywords: dev-doc-needed → dev-doc-complete

Comment 10

7 years ago
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)
Keywords: dev-doc-complete → dev-doc-needed
Changed docs. See bug 62258.
Keywords: dev-doc-needed → dev-doc-complete
Sorry, it's bug 622581
You need to log in before you can comment on or make changes to this bug.