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

RESOLVED FIXED in 0.8

Status

defect
--
minor
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: dbo, Assigned: giermann)

Tracking

unspecified

Details

()

Attachments

(1 attachment)

Reporter

Description

12 years ago
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

12 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

12 years ago
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.
Assignee

Comment 4

12 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 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

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

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