Closed Bug 454766 Opened 11 years ago Closed 11 years ago

Add suggested parentheses to layout/

Categories

(Core :: Layout, defect, minor)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla1.9.1b1

People

(Reporter: Swatinem, Assigned: Swatinem)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
GCC throws a lot of warnings about suggested parentheses. This patch simply adds those parentheses and makes gcc happy :)
Attachment #338076 - Flags: superreview?(roc)
Attachment #338076 - Flags: review?(roc)
-    if (!(aFlags & TRANSFORM_CHANGED) &&
-        svg->mLengthAttributes[nsSVGSVGElement::X].IsPercentage() ||
+    if ((!(aFlags & TRANSFORM_CHANGED) &&
+        svg->mLengthAttributes[nsSVGSVGElement::X].IsPercentage()) ||

This actually looks like buggy code...
Attached patch updated patch (obsolete) — Splinter Review
Yes, you are right. The comment above the if clarifies it. Fixed that case.
Attachment #338076 - Attachment is obsolete: true
Attachment #338621 - Flags: superreview?(roc)
Attachment #338621 - Flags: review?(roc)
Attachment #338076 - Flags: superreview?(roc)
Attachment #338076 - Flags: review?(roc)
Attachment #338621 - Flags: superreview?(roc)
Attachment #338621 - Flags: superreview+
Attachment #338621 - Flags: review?(roc)
Attachment #338621 - Flags: review+
Comment on attachment 338621 [details] [diff] [review]
updated patch

+  if ((aEvent->eventStructType == NS_MOUSE_EVENT &&
+      aEvent->message == NS_MOUSE_BUTTON_UP && 
       static_cast<nsMouseEvent*>(aEvent)->button == nsMouseEvent::eLeftButton) ||

Fix indent.
Indentation fixed. I also moved the "||" to the next line to make that line fit in 80 chars, hope that is ok.
r+ as per comment #3
Attachment #338621 - Attachment is obsolete: true
Attachment #338657 - Flags: superreview+
Attachment #338657 - Flags: review+
Keywords: checkin-needed
Comment on attachment 338657 [details] [diff] [review]
indentation fixed
[Checkin: Comment 4]

http://hg.mozilla.org/mozilla-central/rev/81b6a486f82e

(In reply to comment #4)
> I also moved the "||" to the next line to make that line fit
> in 80 chars, hope that is ok.

I undid that part.
Attachment #338657 - Attachment description: indentation fixed → indentation fixed [Checkin: Comment 4]
Maybe fill a bug for a test for comment 1 !?
Severity: normal → minor
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Keywords: checkin-needed
OS: Linux → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.