In the code for month and multiday views there's an attribute "today" that is not being used

RESOLVED FIXED in 2.2

Status

Calendar
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Decathlon, Assigned: Decathlon)

Tracking

Lightning 1.7

Details

Attachments

(1 attachment, 1 obsolete attachment)

4.88 KB, patch
Decathlon
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
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.
Attachment #667343 - Flags: review?(philipp)
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?
Attachment #667343 - Flags: review?(philipp) → review+
(Assignee)

Comment 3

5 years ago
Created attachment 686514 [details] [diff] [review]
patch - v2

Patch with fix requested in previous comment.
Setting r+ for previous comment
r=philipp
Attachment #667343 - Attachment is obsolete: true
Attachment #686514 - Flags: review+
(Assignee)

Comment 4

5 years ago
Pushed to comm-central changeset 9626be2d64b2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2
You need to log in before you can comment on or make changes to this bug.