926489 - Replace className comparisons with classList.contains in datetimepicker.xml

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

+++ This bug was initially created as a clone of Bug #926487 +++

+++ This bug was initially created as a clone of Bug #926479 +++

+++ This bug was initially created as a clone of Bug #926477 +++

+++ This bug was initially created as a clone of Bug #926471 +++

See the following search for each instance of this: http://mxr.mozilla.org/mozilla-central/search?string=.className+%3D&find=datetimepicker.xml&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

If someone adds a new className to the element, these checks will fail. They should instead be using .classList.contains().

Comment 1

[Soumya Kanti Chakraborty]

I would love to work on the bug. The other bugs mentioned above also has the same issue I believe. Should I go ahead and work for all the above ?

Comment 2

[mjh563]

Assigning to a new contributor by request.

Comment 3

[Akshay Aurora [:system64]]

Attachment: bug-926489-fix.patch

Comment 4

[Akshay Aurora [:system64]]

Attachment: bug-926489-fix-2.patch

Comment 5

[Akshay Aurora [:system64]]

Comment on attachment 822519 [details] [diff] [review]
bug-926489-fix.patch

># HG changeset patch
># User Akshay Aurora <[e-mail]>
># Date 1382734674 -19800
>#      Sat Oct 26 02:27:54 2013 +0530
># Node ID 64427d8f7bf1f4ef650507dee9f4439bb6a21663
># Parent  9f8233fcce1d3f0676bc720303dc7bbd7e246c13
>Bug 926489 - Replace className with classList
>
>diff --git a/toolkit/content/widgets/datetimepicker.xml b/toolkit/content/widgets/datetimepicker.xml
>--- a/toolkit/content/widgets/datetimepicker.xml
>+++ b/toolkit/content/widgets/datetimepicker.xml
>@@ -802,18 +802,17 @@
> 
>               this.yearLeadingZero = (numberFields[yi].length > 1);
>               this.monthLeadingZero = (numberFields[mi].length > 1);
>               this.dateLeadingZero = (numberFields[di].length > 1);
>             }
> 
>             this.yearField = document.getAnonymousElementByAttribute(this, "anonid", yfield);
>             if (!twoDigitYear)
>-              this.yearField.parentNode.className =
>-                "datetimepicker-input-subbox datetimepicker-year";
>+              this.yearField.parentNode.classList.add("datetimepicker-input-subbox", "datetimepicker-year";
>             this.monthField = document.getAnonymousElementByAttribute(this, "anonid", mfield);
>             this.dateField = document.getAnonymousElementByAttribute(this, "anonid", dfield);
> 
>             this._fieldAMPM.parentNode.collapsed = true;
>             this.yearField.size = twoDigitYear ? 2 : 4;
>             this.yearField.maxLength = twoDigitYear ? 2 : 4;
>           ]]>
>         </body>
>@@ -1173,17 +1172,17 @@
> 
>     <handlers>
>       <handler event="click">
>         <![CDATA[
>           if (event.button != 0 || this.disabled || this.readOnly)
>             return;
> 
>           var target = event.originalTarget;
>-          if (target.className == "datepicker-gridlabel" &&
>+          if (target.className.contains("datepicker-gridlabel") &&
>               target != this.selectedItem) {
>             this.selectedItem = target;
>             this._dateValue = new Date(this._displayedDate);
>             if (this.attachedControl)
>               this.attachedControl._setValueNoSync(this._dateValue);
>             this._fireEvent("change", this);
> 
>             if (this.attachedControl && "open" in this.attachedControl)

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 822525 [details] [diff] [review]
bug-926489-fix-2.patch

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

Looks good! I'm forwarding the review request to a toolkit reviewer, but I expect it to get r+ from them. When this gets checked in it may take a couple days before the changes appear within Nightly builds of Lightning.

Comment 7

[mjh563]

The patch has r+, so setting the checkin-needed keyword.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/f80db02ac96d

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f80db02ac96d