Closed
Bug 415990
Opened 16 years ago
Closed 16 years ago
js warning: while(calGoItem = calGoPopupMenu.firstChild) {...
Categories
(Calendar :: General, defect)
Calendar
General
Tracking
(Not tracked)
RESOLVED
FIXED
0.8
People
(Reporter: dbo, Assigned: giermann)
References
()
Details
Attachments
(1 file)
1.67 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
Warning: test for equality (==) mistyped as assignment (=)? Source File: chrome://lightning/content/messenger-overlay-sidebar.js Line: 619, Column: 47 Source Code: while(calGoItem = calGoPopupMenu.firstChild) {
Reporter | ||
Comment 1•16 years ago
|
||
To me that looks like an endless loop. Maybe I miss some some side effect (that I'd like to see documented then).
Assignee | ||
Comment 2•16 years ago
|
||
I think this should remove the warning: - while(calGoItem = calGoPopupMenu.firstChild) { + while((calGoItem = calGoPopupMenu.firstChild) != null) {
Comment 3•16 years ago
|
||
Just to remove the warning, it is sufficient to do: while((calGoItem = calGoPopupMenu.firstChild)) but we should investigate why this is an endless loop.
Assignee | ||
Comment 4•16 years ago
|
||
(In reply to comment #3) > but we should investigate why this is an endless loop. No, this is no endless loop - but since already Berend and now Daniel wondered about it, I should add an comment.
Attachment #302114 -
Flags: review?(philipp)
Comment 5•16 years ago
|
||
Comment on attachment 302114 [details] [diff] [review] Cleanup patch for bug #413620 >- while(calGoItem = calGoPopupMenu.firstChild) { >+ while((calGoItem = calGoPopupMenu.firstChild) != null) { >+ // appendChild actually moves the items from the cloned Node to Thunderbird's menu > tbGoPopupMenu.appendChild(calGoItem); > } I see, interesting behavior but not very intuitive. To make this more intuitive, would something like: while ((calgoItem = calgoPopupMenu.firstChild)) { tbGoPopupMenu.appendChild(calGoPopupMenu.removeChild(calGoItem)); } make sense? I'm fine with both though, its up to you. r=philipp with a decision on which method you prefer.
Attachment #302114 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 6•16 years ago
|
||
(In reply to comment #5) > while ((calgoItem = calgoPopupMenu.firstChild)) { > tbGoPopupMenu.appendChild(calGoPopupMenu.removeChild(calGoItem)); > } I'm fine with both, too. Maybe it's a question of runtime, but I don't want to step too deep into the code of 'appendChild' and it's not too dependent for that - so you're probably right: this would be more intuitive understood on the first look. Since this bug is not assigned to me, I'm unable to set the keyword 'checkin-needed', but please check-in your version with attention to the the spelling: - while(calGoItem = calGoPopupMenu.firstChild) { - tbGoPopupMenu.appendChild(calGoItem); + while((calGoItem = calGoPopupMenu.firstChild)) { + tbGoPopupMenu.appendChild(calGoPopupMenu.removeChild(calGoItem)); }
Updated•16 years ago
|
Assignee: nobody → giermann
Target Milestone: --- → 0.8
Comment 7•16 years ago
|
||
Checked in on HEAD and MOZILLA_1_8_BRANCH -> FIXED
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•