Last Comment Bug 542406 - readonly="true" doesn't work on fields
: readonly="true" doesn't work on fields
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: XBL (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.3a1
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-26 18:40 PST by Blair McBride [:Unfocused] (UNAVAILABLE)
Modified: 2012-03-24 10:40 PDT (History)
7 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (2.97 KB, patch)
2010-01-27 09:45 PST, Boris Zbarsky [:bz]
jonas: review+
Details | Diff | Splinter Review

Description Blair McBride [:Unfocused] (UNAVAILABLE) 2010-01-26 18:40:25 PST
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] 2010-01-26 23:45:05 PST
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) 2010-01-26 23:49:31 PST
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] 2010-01-27 02:53:41 PST
(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 [:bz] 2010-01-27 09:08:35 PST
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] 2010-01-27 09:21:59 PST
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 [:bz] 2010-01-27 09:45:06 PST
Created attachment 423811 [details] [diff] [review]
Fix
Comment 7 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-01-27 10:36:31 PST
Comment on attachment 423811 [details] [diff] [review]
Fix

Thanks bz!
Comment 8 Boris Zbarsky [:bz] 2010-02-03 13:18:41 PST
Pushed http://hg.mozilla.org/mozilla-central/rev/6de23691694a
Comment 10 Nickolay_Ponomarev 2010-03-16 04:23:30 PDT
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 [:fscholz] (MDN) 2012-03-24 10:39:42 PDT
Changed docs. See bug 62258.
Comment 12 Florian Scholz [:fscholz] (MDN) 2012-03-24 10:40:47 PDT
Sorry, it's bug 622581

Note You need to log in before you can comment on or make changes to this bug.