Closed Bug 349322 Opened 14 years ago Closed 14 years ago

calMonthGridPrinter.js outputs invalid and ugly css

Categories

(Calendar :: Printing, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Sunbird 0.3

People

(Reporter: mattwillis, Assigned: mattwillis)

Details

(Keywords: polish)

Attachments

(1 file, 1 obsolete file)

The HTML that calMonthGridPrinter creates has ugly, in some cases invalid, and difficult to read CSS.
Taking.
Status: NEW → ASSIGNED
Target Milestone: --- → Sunbird 0.3
Attached patch rev0 - cleans up CSS (obsolete) — Splinter Review
This patch:
  - normalizes all css to use lowercase
  - adds spaces to make things more readable
  - replaces "background" with "background-color" when only a colour is defined
  - doesn't attempt to include a border on events if the event has no category, or the category has no color assigned
Attachment #234596 - Flags: second-review?(jminta)
Attachment #234596 - Flags: first-review?
Attachment #234596 - Flags: first-review? → first-review?(bugzilla)
Comment on attachment 234596 [details] [diff] [review]
rev0 - cleans up CSS

-            var catColor;
+            var catColor = "";
             try {
                 catColor = pb2.getCharPref("calendar.category.color."+item.getProperty("CATEGORIES").toLowerCase());
             } catch(ex) {}
 
-            var style = 'font-size:11px; background-color:' + calColor + 
-                        '; border: solid ' + catColor + ' 2px;';
+            var style = 'font-size: 11px; background-color: ' + calColor + ';';
+            if (catColor != "") {
+                style += ' border: solid ' + catColor + ' 2px;';
+            }
This looks like there's more than just style cleanup here.  Why are switching from null to an empty string here?
Fixes jminta's nit per IRC chat
Attachment #234596 - Attachment is obsolete: true
Attachment #234810 - Flags: second-review?(jminta)
Attachment #234810 - Flags: first-review?
Attachment #234596 - Flags: second-review?(jminta)
Attachment #234596 - Flags: first-review?(bugzilla)
Attachment #234810 - Flags: first-review? → first-review?(bugzilla)
Comment on attachment 234810 [details] [diff] [review]
rev1 - no more empty string

r2=jminta
Attachment #234810 - Flags: second-review?(jminta) → second-review+
I will only be able to review this on Monday at the earliest. If that is fine with you then leave the request as it is, if you don't want to wait so long, please move it to another reviewer.
Attachment #234810 - Flags: first-review?(bugzilla) → first-review?(cmtalbert)
Attachment #234810 - Flags: first-review?(cmtalbert) → first-review+
Patch checked in on MOZILLA_1_8_BRANCH and trunk.

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