Closed
Bug 14445
Opened 25 years ago
Closed 22 years ago
[RBTN]<input type="radio"> not mutually exclusive without form
Categories
(Core :: Layout: Form Controls, defect, P1)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
mozilla1.0.1
People
(Reporter: ian, Assigned: john)
References
()
Details
(4 keywords, Whiteboard: relnote-devel,DIGBug[HTML4-17.2.1][FIX])
Attachments
(4 files, 10 obsolete files)
198 bytes,
text/html
|
Details | |
243 bytes,
text/html
|
Details | |
27.70 KB,
patch
|
rods
:
review+
jst
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
697 bytes,
patch
|
timeless
:
review+
jst
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
Radio buttons that have the same 'name' attribute but are not in a form are not being considered as mututally exclusive. Clicking on one doesn't unselect the others. This works fine in IE5. <div> <label>One: <input name="x" value="1" type="radio"> </label> <label>Two: <input name="x" value="2" type="radio"> </label> <label>Three: <input name="x" value="2" type="radio"> </label> </div> The following test case shows this: http://www.bath.ac.uk/%7Epy8ieh/internet/projects/mozilla/widgets/03.html TO REPRODUCE: Open up that URI. Click on "One". Click on "Two". ACTUAL RESULT: Both radio buttons are selected. EXPECTED RESULT: Only one radio button should ever be selected at any one time. If the <input> elements are wrapped in a form, then it works fine. A form should not be needed for this -- <form> elements are only wrappers for deciding what fields to send to CGI scripts.
Updated•25 years ago
|
Assignee: karnaze → kmcclusk
Comment 1•25 years ago
|
||
Kevin, I don't think this worked before gfx form controls. Check to see what Nav4.6 does and that's probably all we should do for beta.
Reporter | ||
Comment 2•25 years ago
|
||
Nav 4.x doesn't display the <input> elements _at all_ if they are not in a form, so trying to emulate Nav 4 is probably a very bad idea.... ;-)
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M13
Comment 3•25 years ago
|
||
Moving to M13
Updated•25 years ago
|
Target Milestone: M13 → M15
Comment 5•25 years ago
|
||
Moving to M15
Updated•25 years ago
|
Assignee: kmcclusk → rods
Status: ASSIGNED → NEW
Comment 6•25 years ago
|
||
Reassigning to Rod.
Updated•25 years ago
|
Severity: normal → enhancement
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → LATER
Comment 7•25 years ago
|
||
The spec does not require that this work without a form. changing to LATER and ENHANCEMENT
Reporter | ||
Comment 8•25 years ago
|
||
The spec doesn't say it has to work _in_ a form, either -- it just says: # Radio buttons are like checkboxes except that when several share the same # control name, they are mutually exclusive: when one is switched "on", all # others with the same name are switched "off". - http://www.w3.org/TR/html4/interact/forms.html#h-17.2.1 Note that this works fine in IE. rods: How difficult would this be to implement? Why do we currently _need_ a form element for it to work?
Comment 9•25 years ago
|
||
Yes, we do need the form. Right now the named radio "groupings" are keep in the form, and at this time there is no other "easy" or logical place to put it. So it would take a little bit of work with not much return.
Reporter | ||
Comment 10•25 years ago
|
||
Ok. Do you want to look at this for 5.0 at all? i.e., shall I reopen and mark it as REMIND rather than LATER? Or shall we wait till 5.1?
Comment 11•25 years ago
|
||
It's probably a 5.1 issue.
Reporter | ||
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Reporter | ||
Comment 12•25 years ago
|
||
Ok, 5.1 it is.
Updated•25 years ago
|
Severity: enhancement → normal
Comment 13•25 years ago
|
||
Not to belabor this issue, but I just wanted to make a point in terms of standards compliance. Per the HTML 4.0 Transitional DTD, INPUT is not required to be contained in a FORM. A radio button, which is a value of the 'type' attribute of the INPUT element, should function regardless of whether there is a FORM or not. Therefore, in the case where INPUT is not contained in a FORM, we are not currently standards compliant with regards to the function of radio buttons. Based on this, I am changing the severity from 'enhancement' to 'normal'. As indicated by Rod, resolution is LATERED as priority for fix is low.
Reporter | ||
Comment 14•24 years ago
|
||
Reopening and moving to Future, since "LATER" is now deprecated. rods: Sorry if you didn't want me to do that; feel free to relater it if you don't like the Future milestone.
Comment 15•24 years ago
|
||
What you would seem to want here is to attach all unFORMed input elements in the document to some sort of dummy FORM element. That way you could check for radio buttons with the same name attribute in the same way as you do with any other FORM.
Comment 16•24 years ago
|
||
Future is fine, but I want to bring this in a little, changing to M18
Target Milestone: Future → M17
Comment 17•24 years ago
|
||
Yes, XUL creates a "hidden" "dummy" form to place all form elements. I need to do this for HTML forms elements.
Comment 18•24 years ago
|
||
This bug is marked "future" because it is not critical for RTM (Release To Manufacturing). If anyone believes it is critical, please explain why in this bug.
Status: REOPENED → ASSIGNED
Target Milestone: M17 → Future
Reporter | ||
Comment 19•24 years ago
|
||
Well, one reason to have this fixed by first release is that we would be breaking standards compliance if we did not, and IE has been doing it right for ages. Personally I would say this was an nsbeta3-, FCS-blocker bug, but that's just me... cc'ing Eric Krock for standards compliance judgement call.
Comment 20•24 years ago
|
||
Well, this didn't work in Nav4, it's only permitted for transitional and not in strict (correct me if I'm wrong in interpreting previous comments this way), and there's a simple workaround: enclose your radio buttons in a form element, and then it will work in both IE5 and Mozilla. Unless someone can provide examples of major sites that use this structure, I think we can live with release noting this due to press of other urgent work. Marking relnote, helpwanted, leaving FUTURE.
Keywords: helpwanted,
relnote
Comment 21•24 years ago
|
||
Yes, if you wrap it in a form it works fine, release note it
*** Bug 41430 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 24•24 years ago
|
||
Taking QA per managerial policy.
Reporter | ||
Updated•24 years ago
|
QA Contact: bsharma → py8ieh=bugzilla
Updated•24 years ago
|
Summary: <input type="radio"> not mutually exclusive without form → [RBTN]<input type="radio"> not mutually exclusive without form
Updated•24 years ago
|
Whiteboard: relnote-devel
Updated•24 years ago
|
Priority: P3 → P1
Target Milestone: Future → ---
Comment 25•24 years ago
|
||
This should really be fixed for this next release, leaving as a P1
Comment 26•24 years ago
|
||
Nom. nsbeta1. There is a workaround (modifying the content), but it's confusing as heck for the user to encounter a non-exclusive radio button, so we should fix this to gracefully handle the markup out there on the web.
Keywords: nsbeta1
Comment 27•24 years ago
|
||
I have faced a big problem about radio button mutual exclusion in netscape 6. The event-handling routine(e.g. onclick event) is handled before the update of status of the last selected radio button. Therefore, at the moment while javascript code is running in the scope of onclick event. Both the last selected radio and the newly selected radio are declared themselves as selected. Is there any method to solve it ? We need it urgently. Here is the sample code, please try ! <HTML> <HEAD> <TITLE> Netscape 6 Radio Button Mutual Exclusion</TITLE> </HEAD> <BODY BGCOLOR="#FFFFFF"> <Script language=javascript> function showradio() { for (var i=0; i<document.testradio.test.length; i++) if (document.testradio.test[i].checked) alert("Radio button " + document.testradio.test [i].value + " is selected !"); } </script> Instruction : Please click 1 , 2 , 3 several and you will see the problem. <form name=testradio> <input type=radio name=test value=1 onclick=showradio()>1 <input type=radio name=test value=2 onclick=showradio()>2 <input type=radio name=test value=3 onclick=showradio()>3 </form> </BODY> </HTML>
Comment 28•24 years ago
|
||
This bug bug here is about using radiobuttons without an enclosing form. The bug you are expereincing has already been fixed. I don't remember the bug number, there is no work around. You just need to get a more recent build, then 6.0 production release.
Comment 29•24 years ago
|
||
I think we have some mis-understanding between me and rods. I have add <input type=radio> tag between <Form> but still not work. I use the Netscape 6 Production Release but still get the same result.
Comment 30•24 years ago
|
||
Rod is saying to use a new Mozilla nightly, *not* Netscape 6.
Reporter | ||
Updated•24 years ago
|
Keywords: mozilla0.9,
mozilla1.0
Comment 32•23 years ago
|
||
*** Bug 94116 has been marked as a duplicate of this bug. ***
Comment 33•23 years ago
|
||
*** Bug 99352 has been marked as a duplicate of this bug. ***
Comment 34•23 years ago
|
||
*** Bug 99353 has been marked as a duplicate of this bug. ***
Comment 35•23 years ago
|
||
Wow, going on 2.5 years for this bug to get fixed... *VERY* impressive. Can we maybe get this fixed sometime soon? Here's my view on this: As DHTML and JavaScript become more and more prevalent, the utility of form controls *outside* of forms increases (i.e. since a lot of logic occurs on the client, it's no longer necessary to actually submit a form. Instead, just get some info from form controls in the UI and process it using javascript.) However, if I *have* to embed this in a form tag, I now pay a UI hit since I get all that nasty white space that a form tag forces (as a block element). This is a major nuisance and makes using radios a lot harder in DHTML. :-(
Assignee | ||
Comment 36•23 years ago
|
||
This will happen during or after bug 108308. Setting dependent.
Comment 37•23 years ago
|
||
*** Bug 121309 has been marked as a duplicate of this bug. ***
Updated•23 years ago
|
Whiteboard: relnote-devel → relnote-devel,DIGBug
Assignee | ||
Comment 38•23 years ago
|
||
Taking, leaving milestone set. Whether this gets done with bug 108308 now depends on how easy it is (it may still be easy).
Assignee | ||
Comment 39•22 years ago
|
||
My changes in bug 108308 will make this a lot easier than it was, but there are performance issues to work out before we finally decide how to do it. See comments in that bug for why it was not considered a good idea just yet.
Comment 40•22 years ago
|
||
jkeiser, I'm interesting on this bug. I hacked codes of radio button and find it will not uncheck other buttons when it is be checked because there is no mForm. Do you have any good idea to resolve it? I think maybe we can give all radio buttons(or all formControls) an anymouse form which are not in a form. Do you agree?
Comment 41•22 years ago
|
||
Comment 42•22 years ago
|
||
Comment 43•22 years ago
|
||
This patch works fine with attachment 81144 [details] but not works with attachment 81146 [details].In second test case, all radio button will be selected after page loaded. I guess the reason is, in the second test case, those radio button is in a table. When DoneCreatingElement in nsHTMLInputElement, The table element has not created in content tree. So I can't iterate to table element and of course can't uncheck those radio button. jkeiser, do you have any idea on this problem?
Attachment #81144 -
Attachment is patch: false
Attachment #81144 -
Attachment mime type: text/plain → text/html
Comment 44•22 years ago
|
||
jkeiser, per discussion on IRC, I get a new patch here. Please take a look. I added a hashtable to document, And implemented it in nsDocument and nsXULDocument. Also, I found a bug(maybe a bug) in nsDoubleHashtable.h, it cause redefine error when I want to use the hashtable in nsXULDocument. This patch also include fix of form's radio walker and visitor. There is another issue that when page reload, the value of radio group not remembered. I think need to add something when the element restore and save it's state.
Attachment #81148 -
Attachment is obsolete: true
Comment 45•22 years ago
|
||
jst, jkeiser When I was hacking on this bug, I found there is a mContentWrapperHash in nsDocument. It's for nsIDocument::AddReference and nsIDocument::RemoveReference. I don't know it's usage. Fabian told me that maybe it is dead code. How do you think about it?
Comment 46•22 years ago
|
||
Pete, that code is used by the nsDOMClassInfo code to ensure rooting (in JS GC terms) of content wrappers when non-predefined properties are set on DOM nodes. It is not dead code...
Comment 47•22 years ago
|
||
per discussion with jkeiser on IRC, I have worked out a new patch. jkeiser, this patch include you comments, please review. It also fix some other problems. 1) SetType in nsHTMLInputElement should call it's own SetAttr instead of nsGenericHTMLLeafFormElement's 2) We should remove/add radio to/from group if it's type changed - if (aName == nsHTMLAtoms::name && + if ((aName == nsHTMLAtoms::name || aName == nsHTMLAtoms::type) && I also added some comments, but I'm not sure they are enough, please check. Thanks
Attachment #81279 -
Attachment is obsolete: true
Assignee | ||
Comment 48•22 years ago
|
||
Comment on attachment 83529 [details] [diff] [review] patch(let document support radio button) Most of the changes below are localized, but there are enough that I'd like to see another patch. r=jkeiser with these fixed. >Index: base/public/nsIDocument.h >=================================================================== >+ NS_IMETHOD SetCurrentRadioButton(const nsAString& aName, >+ nsISupports* aRadio) = 0; >+ >+ NS_IMETHOD GetCurrentRadioButton(const nsAString& aName, >+ nsISupports** aRadio) = 0; Spacing. Also all these methods needs comments. All new methods (or changed methods) require JavaDoc-like comments (@param and @return and all that). @return is not necessary to document for NS_IMETHOD or nsresult, of course. Also, it would mimick the form interface better and would be clearer if these were passed and stored as nsIDOMHTMLInputElement. >Index: base/src/nsDocument.cpp >=================================================================== >+nsresult >+nsDocument::GetRadioGroup(const nsAString& aName, >+ nsRadioGroupStruct **aRadioGroup) ... >+ if (!radioGroup) return NS_ERROR_OUT_OF_MEMORY; NS_ENSURE_TRUE(radioGroup, NS_ERROR_OUT_OF_MEMORY) will get you out of the "always use brackets" rule :) >+NS_IMETHODIMP >+nsDocument::SetCurrentRadioButton(const nsAString& aName, >+ nsISupports* aRadio) >+{ >+ nsRadioGroupStruct* radioGroup = nsnull; >+ GetRadioGroup(aName, &radioGroup); >+ if (radioGroup) { >+ radioGroup->mSelectedRadioButton = aRadio; >+ NS_IF_ADDREF(radioGroup->mSelectedRadioButton); Leak. Need to release the current radio button before you set it, since you addref it afterwards. >+NS_IMETHODIMP >+nsDocument::GetCurrentRadioButton(const nsAString& aName, >+ nsISupports** aRadio) >+{ >+ nsRadioGroupStruct* radioGroup = nsnull; >+ GetRadioGroup(aName, &radioGroup); >+ if (radioGroup) >+ *aRadio = radioGroup->mSelectedRadioButton; You *must* addref the result here. Getters return strong references, and this is no exception. >+NS_IMETHODIMP >+nsDocument::RemoveFromRadioGroup(const nsAString& aName, >+ nsISupports* aRadio) >+{ >+ nsRadioGroupStruct* radioGroup = nsnull; >+ GetRadioGroup(aName, &radioGroup); >+ if (radioGroup) >+ radioGroup->mRadioButtons.RemoveElement(aRadio); Leak. Need to release the element too, since you addref'd when you did AddToRadioGroup. >+NS_IMETHODIMP >+nsDocument::WalkRadioGroup(const nsAString& aName, >+ nsIRadioVisitor* aVisitor) >+{ ... >+ nsCOMPtr<nsIFormControl> control; >+ control = do_QueryInterface(NS_STATIC_CAST(nsISupports *, >+ radioGroup->mRadioButtons.ElementAt(i))); If you stored nsIFormControl in the array instead of nsISupports, and passed that in to the Add/Remove functions instead of nsISupports, that would get rid of this QueryInterface. Less QI's is better, they are expensive. >Index: html/content/src/nsHTMLInputElement.cpp >=================================================================== > NS_IMETHOD HandleDOMEvent(nsIPresContext* aPresContext, nsEvent* aEvent, > nsIDOMEvent** aDOMEvent, PRUint32 aFlags, > nsEventStatus* aEventStatus); >+ >+ NS_IMETHOD SetDocument(nsIDocument* aDocument, PRBool aDeep, >+ PRBool aCompileEventHandlers); >+ > #ifdef DEBUG > NS_IMETHOD SizeOf(nsISizeOfHandler* aSizer, PRUint32* aResult) const; > #endif >@@ -414,7 +418,7 @@ > const nsAString* aValue, > PRBool aNotify) > { >- if (aName == nsHTMLAtoms::name && >+ if ((aName == nsHTMLAtoms::name || aName == nsHTMLAtoms::type) && > mType == NS_FORM_INPUT_RADIO) { > // XXX Because we are not doing anything that assumes the radio button has > // actually had its name changed at this point, we can get away with >@@ -433,7 +437,7 @@ > // When name changes, radio moves to a different group > // (type changes are handled in the form itself currently) > // >- if (aName == nsHTMLAtoms::name && >+ if ((aName == nsHTMLAtoms::name || aName == nsHTMLAtoms::type) && > mType == NS_FORM_INPUT_RADIO) { > AddedToRadioGroup(); > } Type changes are handled in the form, so you only want to call AddedToRadioGroup when type changes and the radio button is not in a form. > // This is only initialized in if (mForm) for use in later if (mForm)s Need to update this comment for document also, it really should be just after the declaration of nsAutoString name;. >@@ -833,9 +844,12 @@ > // > // Let the form know that we are now the One True Radio Button > // Comment update (s/form/form and document). >- if (mForm && NS_SUCCEEDED(rv)) { >+ NS_ENSURE_SUCCESS(rv, rv); >+ if (mForm) > rv = mForm->SetCurrentRadioButton(name, NS_STATIC_CAST(nsIDOMHTMLInputElement*, this)); >- } >+ else if (mDocument) >+ rv = mDocument->SetCurrentRadioButton(name, NS_STATIC_CAST(nsISupports *, >+ NS_STATIC_CAST(nsIDOMHTMLInputElement*, this))); Put brackets around if's please. >+NS_IMETHODIMP >+nsHTMLInputElement::SetDocument(nsIDocument* aDocument, PRBool aDeep, >+ PRBool aCompileEventHandlers) >+{ ... >+ if (GET_BOOLBIT(mBitField, BF_PARSER_CREATING)) >+ return NS_OK; Need a comment there, that is counterintuitive even though it is the right thing to do. Also, you want to return early if the radio button is in a form; otherwise you will add the radio button twice. Braces around the if too. >+ if (aDocument) >+ AddedToRadioGroup(); >+ else >+ RemovedFromRadioGroup(nsnull, nsnull); >+ Braces around if. >@@ -2365,6 +2399,9 @@ > } > > SET_BOOLBIT(mBitField, BF_SHOULD_INIT_CHECKED, PR_FALSE); >+ >+ if (!mForm && mType == NS_FORM_INPUT_RADIO) >+ AddedToRadioGroup(); > > return NS_OK; > } Comment. > nsHTMLInputElement::AddedToRadioGroup() > { > // >- // Currently the only time radio groups really happen is in forms >- // >- if (!mForm) { >- return NS_OK; >- } Doesn't matter too much, but you may as well just change that to if (!mForm && !mDocument) so that radios not in a document / form don't get penalized. >+ if (!mForm && mDocument) { >+ nsAutoString name; >+ GetName(name); >+ mDocument->AddToRadioGroup(name, NS_STATIC_CAST(nsISupports *, >+ NS_STATIC_CAST(nsIDOMHTMLInputElement*, this))); >+ } >+ Comment here, and it's probably best if it is added after SetCheckedChangedInternal(), because that code is logically grouped with the VisitGroup() stuff above. > nsHTMLInputElement::RemovedFromRadioGroup(nsIForm* aForm, nsAString* aName) > { > // >- // Currently radio groups only happen in forms >- // >- if (!aForm) { >- return NS_OK; >- } Same as in AddedToRadioGroup, if (!mForm && !mDocument) and update comment. >+ if (aName) >+ name = *aName; This halfway defeats the purpose of passing in aName--this operation performs a string copy. To make the code easier to read, you could always use *aName but when you do GetName(name) set aName = &name; *Make sure name has a greater scope than aName in that case.* > nsresult > nsHTMLInputElement::VisitGroup(nsIRadioVisitor* aVisitor) > { ... >+ mDocument->WalkRadioGroup(name, aVisitor); If you do rv = mDocument->WalkRadioGroup() like above, you don't have to initialize rv at the top. >Index: xul/document/src/nsXULDocument.cpp >=================================================================== nsXULDocument is going to need this, too, but you can just make a separate bug for it. Eventually XULDocument will extend nsDocument, so will probably never have to worry about it.
Comment 49•22 years ago
|
||
I will file another bug to implement functions of nsIDocument in nsXULDocument later
Attachment #83529 -
Attachment is obsolete: true
Assignee | ||
Comment 50•22 years ago
|
||
Comment on attachment 83686 [details] [diff] [review] patch (with jkeiser's comments) One small new thing (sorry I missed it before) ... instead of typedef struct { ... } name; use struct <name> { ... }; It's what we do in most places. You missed the first review comment in nsHTMLInputElement ... about not calling AddedToRadioGroup if the radio button is in a form and the type changes. In the comment in SetDocument you should say "and the parser is still creating the element" instead of "BF_PARSER_CREATING is true", which doesn't tell the reader much that they didn't already know :) Also you didn't make a comment near the end of DoneCreatingElement to say why you are adding to radio group ... need to explain that the radio button may have been added to the document while the parser was still creating the element but we did not want to waste time adding it to the group until everything was done. r=jkeiser with these, so especially since they are all comment fixes (except the typedef struct thing) go ahead and go straight to sr stage when you get them done :)
Assignee | ||
Comment 51•22 years ago
|
||
Erk, the second comment there is also a necessary code change, shouldn't be big though (just adding "&& !mForm")
Comment 52•22 years ago
|
||
Attachment #83686 -
Attachment is obsolete: true
Comment 53•22 years ago
|
||
Fix this: Don't AddedToRadioGroup/RemovedFromRadioGroup before parser DoneCreatingElement or radio is in a form and type changes jst, this patch is ready for sr=, please go ahead. Thanks!
Attachment #83693 -
Attachment is obsolete: true
Assignee | ||
Comment 54•22 years ago
|
||
Comment on attachment 83695 [details] [diff] [review] patch(fix some problem) r=jkeiser
Attachment #83695 -
Flags: review+
Comment 55•22 years ago
|
||
Comment on attachment 83695 [details] [diff] [review] patch(fix some problem) The more I think about this, the more I think that putting these methods in nsIDocument is not a good idea. Since we now have some radio control related methods duplicated and some methods that are very similar shared between nsIDocument and nsIForm, seems to me that we're in need of something like a nsIRadioControlContainer interface or somesuch. Pete, would you want to take a stab at that or do you want me or jkeiser to? Also, calls to GetRadioGroup() in nsDocument can fail (OOM) so you should propagate the return value to the callers... Other than that the changes look reasonable (didn't look all that close yet though).
Attachment #83695 -
Flags: needs-work+
Comment 56•22 years ago
|
||
jst, I have already return it like this NS_ENSURE_TRUE(radioGroup, NS_ERROR_OUT_OF_MEMORY); (The jst-reviewer.pl need patch :-)) I think you are right. I discussed with jkeiser about this before and we all think it's a cool idea. But it need more time and more thinking. I'm not sure I have much time to do it recently because I'm also working on accessibility stuff... But, it's ok, I will do it when I have time and if anyone have time and interesting also can help.
Assignee | ||
Comment 57•22 years ago
|
||
*** Bug 145647 has been marked as a duplicate of this bug. ***
Comment 58•22 years ago
|
||
*** Bug 154472 has been marked as a duplicate of this bug. ***
Comment 59•22 years ago
|
||
Can someone please add the word "radiobutton" to the short description or the keywords, as that's how it's commonly called and that's what people will search for (I did as well, didn't find it and thus caused a new duplicate to that bug). It's really important to use the terms in the short description or in the keywords that the user will use when searching for the bug. And searching for the HTML name of that tag where the last thing I had ever thought about. It is a radiobutton (also named that way in programming languages), hence that's what people will look for.
Assignee | ||
Comment 60•22 years ago
|
||
This patch doesn't work yet but has all the ingredients it needs. Posting it here as a transport mechanism between home and work :)
Assignee | ||
Comment 61•22 years ago
|
||
Yet another transfer patch. Works fine for normal radios but on testcases here, crashy crashy.
Attachment #90972 -
Attachment is obsolete: true
Updated•22 years ago
|
Whiteboard: relnote-devel,DIGBug → relnote-devel,DIGBug[HTML4-17.2.1]
Comment 62•22 years ago
|
||
*** Bug 157405 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 63•22 years ago
|
||
This should do it. Changes: - moves the generic radio group functions from nsIDocument into nsIRadioGroupContainer (unfortunately there are a few that didn't fit into forms as well--AddToRadioGroup and RemoveFromRadioGroup--so they stayed) - changes RemovedFromRadioGroup to WillRemoveFromRadioGroup to simplify things - fixed a few dynamic JS bugs (it's now fully tested on dynamic JS, I believe) To test this stuff, go to http://www.johnkeiser.com/mozilla/radio/jsadd.html. The second group of radios is outside of a form, and I think you can test just about every imaginable JS operation, including moving radios from inside a form to outside a form and back. Let me know if you can think of more.
Attachment #91074 -
Attachment is obsolete: true
Assignee | ||
Comment 64•22 years ago
|
||
I want to point out that this patch was based on Pete Zha's working patch from before.
Status: NEW → ASSIGNED
Whiteboard: relnote-devel,DIGBug[HTML4-17.2.1] → relnote-devel,DIGBug[HTML4-17.2.1][FIX]
Assignee | ||
Comment 65•22 years ago
|
||
This patch is identical to the previous one except that it moves AddToRadioGroup and RemoveFromRadioGroup into nsIRadioGroupContainer, which does in fact save us some code. Forms just return NS_OK from these methods.
Attachment #83695 -
Attachment is obsolete: true
Attachment #91445 -
Attachment is obsolete: true
Comment 66•22 years ago
|
||
Comment on attachment 91512 [details] [diff] [review] Patch v1.3.1 r=rods
Attachment #91512 -
Flags: review+
Comment 67•22 years ago
|
||
Comment on attachment 91512 [details] [diff] [review] Patch v1.3.1 sr=jst
Attachment #91512 -
Flags: superreview+
Comment 68•22 years ago
|
||
Comment on attachment 91512 [details] [diff] [review] Patch v1.3.1 a=asa (on behalf of drivers) for checkin to 1.1
Attachment #91512 -
Flags: approval+
Assignee | ||
Comment 69•22 years ago
|
||
Checked in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 22 years ago
Resolution: --- → FIXED
Comment 70•22 years ago
|
||
The following chunk of the nsHTMLInputElement.cpp changes looks pretty suspicious: @@ -2573,13 +2688,14 @@ nsHTMLInputElement::VisitGroup(nsIRadioVisitor* aVisitor) { nsresult rv; - if (mForm) { + nsCOMPtr<nsIRadioGroupContainer> container = GetRadioGroupContainer(); + if (container) { nsAutoString name; GetName(name); - rv = mForm->WalkRadioGroup(name, aVisitor); + rv = container->WalkRadioGroup(name, aVisitor); } else { - PRBool stop = PR_FALSE; - rv = aVisitor->Visit(this, &stop); + PRBool stop; + aVisitor->Visit(this, &stop); } return rv; } This way in the "else" case the rv is no longer set to anything (and is returned uninitialized!). P.S. I've noticed this by seeing that a new "may be used uninitialized" warning have appeared on Brad TBox: +content/html/content/src/nsHTMLInputElement.cpp:2690 + `nsresult rv' might be used uninitialized in this function More information on these warnings can be found in bug 59652.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 71•22 years ago
|
||
This bug is fixed.
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 72•22 years ago
|
||
(Open a new bug on that stuff, I'm looking into it now.)
Assignee | ||
Comment 73•22 years ago
|
||
Excellent catch.
Assignee | ||
Comment 74•22 years ago
|
||
Excellent catch.
Assignee | ||
Updated•22 years ago
|
Attachment #92159 -
Attachment is obsolete: true
Assignee | ||
Comment 75•22 years ago
|
||
So don't worry about opening a new bug unless you already have; we'll deal with it in the bug here. For future reference, opening bugs that are fixed is not the right way to do things. It is best to either comment or write a new bug.
Attachment #92158 -
Flags: review+
Comment 76•22 years ago
|
||
Comment on attachment 92158 [details] [diff] [review] Patch For Uninitialized Var sr=jst
Attachment #92158 -
Flags: superreview+
Comment 77•22 years ago
|
||
Comment on attachment 92158 [details] [diff] [review] Patch For Uninitialized Var a=asa (on behalf of drivers) for checkin to 1.1
Attachment #92158 -
Flags: approval+
Assignee | ||
Comment 78•22 years ago
|
||
Fix checked in.
Comment 79•22 years ago
|
||
batch: adding topembed per Gecko2 document http://rocknroll.mcom.com/users/marek/publish/Gecko/Gecko2Tasks.html
Keywords: topembed
Assignee | ||
Comment 80•22 years ago
|
||
*** Bug 150936 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•