1392167 - [Onboarding] have getCurrentActiveTour() check states of all DOM elements

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I read bug 1383505 and found what Fisher asked was quiet trivial. In fact, I've done with in my lunch time. Let me flag both of you for review.

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Bug 1392167 - Have getCurrentActiveTour() check states of all DOM elements.

Review commit: https://reviewboard.mozilla.org/r/170548/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170548/

Comment 2

[Fischer [:Fischer]]

Comment on attachment 8899330 [details]
Bug 1392167 - Have getCurrentActiveTour() check states of all DOM elements.

https://reviewboard.mozilla.org/r/170548/#review175794

Thanks for the wrok. The test is even neater.

::: browser/extensions/onboarding/test/browser/head.js:186
(Diff revision 1)
>      for (let item of items) {
>        if (item.classList.contains("onboarding-active")) {
> +        if (!activeNavItemId) {
> -        activeNavItemId = item.id;
> +          activeNavItemId = item.id;
> -        break;
> +        } else {
> +          ok(false, "There are more than one item marked as active.");

Thank you for doing this check as well.

Comment 3

[[e-mail]]

Comment on attachment 8899330 [details]
Bug 1392167 - Have getCurrentActiveTour() check states of all DOM elements.

https://reviewboard.mozilla.org/r/170548/#review176130

::: browser/extensions/onboarding/test/browser/head.js:186
(Diff revision 1)
>      for (let item of items) {
>        if (item.classList.contains("onboarding-active")) {
> +        if (!activeNavItemId) {
> -        activeNavItemId = item.id;
> +          activeNavItemId = item.id;
> -        break;
> +        } else {
> +          ok(false, "There are more than one item marked as active.");

thanks, that nicely validate the non-active item

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/829183c09b2c
Have getCurrentActiveTour() check states of all DOM elements. r=Fischer,gasolin

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/829183c09b2c