Closed Bug 415990 Opened 13 years ago Closed 13 years ago

js warning: while(calGoItem = calGoPopupMenu.firstChild) {...


(Calendar :: General, defect)

Not set


(Not tracked)



(Reporter: dbo, Assigned: giermann)





(1 file)

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) {
To me that looks like an endless loop. Maybe I miss some some side effect (that I'd like to see documented then).
I think this should remove the warning:

-    while(calGoItem = calGoPopupMenu.firstChild) {
+    while((calGoItem = calGoPopupMenu.firstChild) != null) {
Just to remove the warning, it is sufficient to do:

while((calGoItem = calGoPopupMenu.firstChild))

but we should investigate why this is an endless loop.
(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 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)) {

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+
(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));
Assignee: nobody → giermann
Target Milestone: --- → 0.8
Checked in on HEAD and MOZILLA_1_8_BRANCH

Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.