Last Comment Bug 542406 - readonly="true" doesn't work on fields
: readonly="true" doesn't work on fields
: 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] (still a bit busy)
: Andrew Overholt [:overholt]
Depends on:
  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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Fix (2.97 KB, patch)
2010-01-27 09:45 PST, Boris Zbarsky [:bz] (still a bit busy)
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>, 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] (still a bit busy) 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] (still a bit busy) 2010-01-27 09:45:06 PST
Created attachment 423811 [details] [diff] [review]
Comment 7 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-01-27 10:36:31 PST
Comment on attachment 423811 [details] [diff] [review]

Thanks bz!
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2010-02-03 13:18:41 PST
Comment 10 Nickolay_Ponomarev 2010-03-16 04:23:30 PDT
removing dev-doc-complete: is about a XUL attribute, this bug is about XBL <field readonly="true">. The docs now incorrectly state that e.g. did not work before 1.9.3.

There's also no way to figure out from 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.