Last Comment Bug 797271 - In the code for month and multiday views there's an attribute "today" that is not being used
: In the code for month and multiday views there's an attribute "today" that is...
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: General (show other bugs)
: Lightning 1.7
: All All
: -- normal (vote)
: 2.2
Assigned To: Decathlon
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-02 23:28 PDT by Decathlon
Modified: 2012-11-29 04:42 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (3.09 KB, patch)
2012-10-02 23:42 PDT, Decathlon
philipp: review+
Details | Diff | Splinter Review
patch - v2 (4.88 KB, patch)
2012-11-29 03:21 PST, Decathlon
bv1578: review+
Details | Diff | Splinter Review

Description Decathlon 2012-10-02 23:28:04 PDT
In comm-central/calendar I searched for the string "today" (with double quotes) to see where the attribute could be set, get or removed:

http://mxr.mozilla.org/comm-central/search?find=/calendar/&string=%22today%22

Then I searched for the string "today=" (without double quotes and also variations with spaces) to see where the attribute is being used:

http://mxr.mozilla.org/comm-central/search?find=/calendar/&string=today%3D

The only "useful" attribute "today" is that one used in the minimonth binding (and datepickers that extend that binding) and related css files.
It seems there aren't other code lines where the attribute is being used in the views apart from the following lines:

http://mxr.mozilla.org/comm-central/source/calendar/base/content/calendar-month-view.xml#816
http://mxr.mozilla.org/comm-central/source/calendar/base/content/calendar-multiday-view.xml#3277

that merely set and remove the attribute for the views but no other line where the attribute is being used for any reason.
All the views' styles (.css files) use the "relation" attribute that can get the "today" value, but not directly a "today" attribute.
Comment 1 Decathlon 2012-10-02 23:42:38 PDT
Created attachment 667343 [details] [diff] [review]
patch

There is a few code lines around line 816 that I used for the patch for bug 455045. Since the "today" attribute seems useless, that code seems useless too, so I've also deleted it and relocated elsewhere what it needs.
Comment 2 Philipp Kewisch [:Fallen] 2012-11-08 05:01:24 PST
Comment on attachment 667343 [details] [diff] [review]
patch

Review of attachment 667343 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp with nit fixed.

Could you also do me a favor and add this to your .hgrc ?

[defaults]
qnew = -D -U
qrefresh = -D

It makes sure that the correct user header (what you set in ui.username) is added to patches you create with qnew. Also, for this patch (and other existing patches you have in your queue) could you do hg qref -U so it sticks your username to the patch?

This makes it easier for me to push since I only have to qimport, possibly fix the commit message and push. If you want to make it even easier for me, then format your patch messages like (case sensitive):

hg qref -m "Fix bug 797271 - In the code for month and multiday views there's an attribute \"today\" that is not being used. r=philipp"

::: calendar/base/content/calendar-month-view.xml
@@ +743,5 @@
>              for (var j = 0; j < row.childNodes.length; j++) {
>                var daybox = row.childNodes[j];
>                var date = dateList[dateBoxes.length];
>  
> +              if (i == 0) {

Could you add an inline comment why this is needed?
Comment 3 Decathlon 2012-11-29 03:21:45 PST
Created attachment 686514 [details] [diff] [review]
patch - v2

Patch with fix requested in previous comment.
Setting r+ for previous comment
r=philipp
Comment 4 Decathlon 2012-11-29 03:26:27 PST
Pushed to comm-central changeset 9626be2d64b2

Note You need to log in before you can comment on or make changes to this bug.