926477 - Replace className comparisons with classList.contains in browser-places.js

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

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

+++ 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%3D&find=browser-places.js&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

[waldion]

Can this bug be assigned to me please?

Comment 2

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

Any update on this?

Comment 3

[waldion]

Fixed the bug, running the tests now.

Comment 4

[waldion]

The fix is included in the patch uploaded by [e-mail] (for bug 926487)

Comment 5

[waldion]

Attachment: Replace className comparisons

Comment 6

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

Comment on attachment 8344187 [details] [diff] [review]
Replace className comparisons

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

Can you please add a patch summary of: "Bug 926477: Replace className comparisons with classList.contains in browser-places.js. r=jaws" and reupload?

::: browser/base/content/browser-places.js
@@ +102,5 @@
>                this.cancelButtonOnCommand();
>              break;
>            case KeyEvent.DOM_VK_RETURN:
> +            if (aEvent.target.classList.contains("expander-up") ||
> +                 aEvent.target.classList.contains("expander-down") ||

This looks good, but an extra space got added in the beginning of this line.

Comment 7

[waldion]

Attachment: 926477.patch

Comment 8

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

Comment on attachment 8345311 [details] [diff] [review]
926477.patch

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

This looks good. I had to fix the commit message as all of the patch details got pulled in to the summary somehow.

I've pushed this to the fx-team repository. If all goes as planned, it should be merged to mozilla-central in a day or two, and then the following day the changes will appear in the Nightly build of Firefox. Congratulations on your first patch! :)

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

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/764e877917a4