Open Bug 1443572 Opened 3 years ago Updated 1 year ago

Disable onboarding when using Marionette

Categories

(Testing :: Marionette, enhancement, P3)

Version 3
enhancement

Tracking

(Not tracked)

People

(Reporter: davehunt, Unassigned, Mentored)

Details

(Keywords: good-first-bug)

User Story

Please read the following document in how to get started:

https://firefox-source-docs.mozilla.org/testing/marionette/marionette/NewContributors.html

Attachments

(2 files)

The browser onboarding can be disabled by setting the 'browser.onboarding.enabled' pref to false. This can be added to the RECOMMENDED_PREFS map in testing/marionette/components/marionette.js.
Mentor: dave.hunt
Keywords: good-first-bug
:davehunt May I take this up? Thanks!
Hello,
Is this bug open for solving?
> Is this bug open for solving?

Yes! If you're interested in working on it I can assign it to you. Have you contributed to mozilla-central previously? If not, you may find https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction a useful place to get started.
No, This will be my first time and I would like to work on this bug.
Thank you
Assignee: nobody → akshitadvlp
Status: NEW → ASSIGNED
I have assigned the bug to you. Please comment here if you need any assistance.
Mentor: dave.hunt → hskupin
Thank you for assigning the issue.
Hello,
I had started with the issue and understood the changes I need to make to the repo.
But I got stuck with part of how to implement the changes with the Mozilla-central and also studied the page directed to me but :(
It would be great if I could get a starting on how to proceed.

Thank you and sorry for the convience :)
akshitadvlp, I added a link to our documenation to the user story section. Please read it to know how to get started.
User Story: (updated)
Otherwise you can also reach out on the #introduction or #ateam IRC channels.
akshitadvlp, please see my last comment and the URL I added in the user story section of this bug. When you open it you will find all details under `Work on bugs and get code review`. Just follow the links and read on.
Hello Henrik,
I am following the documentation, thank you for linking that.
I just thought to get confirmation about the changes before pushing them now I will upload it through the review part.
Thank you and sorry for the inconvenience :)
We do reviews through our review tools, just to make sure to track remaining issues correctly. So thanks for uploading now.
Hello,
I have made all the changes as mentioned but when pushing to review through ssh. It is giving me an error.
remote: Permission denied (public key).
abort: no suitable response from remote hg!
Can I get a little suggestion on how to move forward?
Thank you :)
Can you please tell me which commands you used to push the changes? Make sure that you run `hg push review`. I assume you missed to specify `review` which indeed will not work. To solve problems like that it is better to reach us on IRC so that we can clarify this directly.
Hello,
I have submitted a review :)
Comment on attachment 8961996 [details]
bug 1443572

https://reviewboard.mozilla.org/r/230844/#review236332


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Invalid syntax [py3: is-parseable]

::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Invalid syntax [py2: is-parseable]

::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Syntaxerror: invalid syntax [flake8: E999]
Comment on attachment 8961996 [details]
bug 1443572

https://reviewboard.mozilla.org/r/230844/#review236334


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Invalid syntax [py3: is-parseable]

::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Invalid syntax [py2: is-parseable]

::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Syntaxerror: invalid syntax [flake8: E999]
Comment on attachment 8961996 [details]
bug 1443572

https://reviewboard.mozilla.org/r/230844/#review236336


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Invalid syntax [py3: is-parseable]

::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Invalid syntax [py2: is-parseable]

::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Syntaxerror: invalid syntax [flake8: E999]
Comment on attachment 8961996 [details]
bug 1443572

https://reviewboard.mozilla.org/r/230844/#review236338


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Invalid syntax [py3: is-parseable]

::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Invalid syntax [py2: is-parseable]

::: testing/marionette/client/marionette_driver/geckoinstance.py:369
(Diff revision 1)
>          "browser.snippets.enabled": False,
>          "browser.snippets.syncPromo.enabled": False,
>          "browser.snippets.firstrunHomepage.enabled": False,
>  
> +        # (bug 1443572)
> +        "browser.onboarding.enabled", False,

Error: Syntaxerror: invalid syntax [flake8: E999]
akshitadvlp, please check the review comments from the code review bot which you want to fix. Also make sure that you run the Marionette unit tests locally to make sure that the code is working. Right now you will see a couple of exceptions.

Once it has been done and you updated the mozreview patch, make also sure to flag me as reviewer of the patch inside of mozreview. Thanks.
Thank you the guidance. I will implement the changes and revert you asap.
Flags: needinfo?(akshitadvlp)
Priority: -- → P3
The system I had made the setup for Mozilla-central crashed and I have been waiting for it to get repaired to complete the bug.
Flags: needinfo?(akshitadvlp)
I wonder if you will have the time to finish your patch. If not please let us know. Thanks.
Flags: needinfo?(akshitadvlp)
Assignee: akshitadvlp → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(akshitadvlp)
As @whimboo suggested, I'll start working on this bug as soon as I can.
Comment on attachment 8983511 [details]
Bug 1443572 - Disable Photon onboarding experience for Marionette and geckodriver

https://reviewboard.mozilla.org/r/249362/#review255538

I did not know what tests to run, so I was about to run the unit tests, but I needed a build and it was taking a lot of time, so I pushed this to review, so that the try server can find all the errors.

::: testing/geckodriver/src/prefs.rs:47
(Diff revision 1)
>  
>          // Never start the browser in offline mode
>          ("browser.offline", Pref::new(false)),
>  
> +        // Disable browser onboarding
> +        // (bug 1443572)

I am doubtful that this comment is needed, but I saw a similar comment on line 28 and kept it.
Comment on attachment 8983511 [details]
Bug 1443572 - Disable Photon onboarding experience for Marionette and geckodriver

https://reviewboard.mozilla.org/r/249364/#review255718

::: commit-message-89d79:1
(Diff revision 1)
> +Bug 1443572 - Disabled browser onboarding when using Marionette r?whimboo

Please adjust so it looks like:

`Bug 1443572 - Disable Photon onboarding experience for Marionette and geckodriver.`

::: testing/geckodriver/src/prefs.rs:11
(Diff revision 1)
>  // channel builds of Firefox.  Removing a preference from this file
>  // will cause regressions, so please be careful and get review from
>  // a Testing :: Marionette peer before you make any changes to this file.
>  
>  lazy_static! {
>      pub static ref DEFAULT: [(&'static str, Pref); 80] = [

You added a pref so the length of the list also needs to be bumped up.

Please note that I landed huge changes to that file yesterday, which means you have to rebase your bookmark against central, after pulling from it.

Currently you wont be able to compile geckodriver. Try it out yourself by changing into `testing/geckodriver` and call `cargo build`.

::: testing/geckodriver/src/prefs.rs:47
(Diff revision 1)
>  
>          // Never start the browser in offline mode
>          ("browser.offline", Pref::new(false)),
>  
> +        // Disable browser onboarding
> +        // (bug 1443572)

Lets name it "Disable Photon onboarding experience". Also there is no need to add the bug number here because there is nothing left to do.

::: testing/marionette/client/marionette_driver/geckoinstance.py:381
(Diff revision 1)
>      fennec_prefs = {
>          # Enable output of dump()
>          "browser.dom.window.dump.enabled": True,
>  
> +        # Disable browser onboarding
> +        # (bug 1443572)

Photon is Firefox only. So we don't need it for the Fennec instance.

::: testing/marionette/client/marionette_driver/geckoinstance.py:523
(Diff revision 1)
>          # Do not show the EULA notification which can interfer with tests
>          "browser.EULA.override": True,
>  
> +        # Disable browser onboarding
> +        # (bug 1443572)
> +        "browser.onboarding.enabled": False,

Same as for my comment for prefs.rs. Please update.

::: testing/marionette/components/marionette.js:97
(Diff revision 1)
>    // as it is picked up at runtime.
>    ["browser.offline", false],
>  
> +  // Disable browser onboarding
> +  // (bug 1443572)
> +  ["browser.onboarding.enabled", false],

Same as above.
Attachment #8983511 - Flags: review?(hskupin) → review-
As I mentioned on IRC, I do not see "browser.onboarding.enabled" as false, which I have set in the files. I believe I checked what bookmark I was on, and also verified that the files had it set as false. @whimboo mentioned about using unpatched Marionette, but I do not understand how to do that.
Flags: needinfo?(hskupin)
Comment on attachment 8983511 [details]
Bug 1443572 - Disable Photon onboarding experience for Marionette and geckodriver

https://reviewboard.mozilla.org/r/249364/#review256468

::: testing/marionette/client/marionette_driver/geckoinstance.py:518
(Diff revision 2)
>  
>          # Do not show the EULA notification which can interfer with tests
>          "browser.EULA.override": True,
>  
> +        # Disable Photon onboarding experience
> +        "broswer.onboarding.enabled": False,

This should be `browser`.
Attachment #8983511 - Flags: review?(hskupin) → review-
(In reply to Aditya Khadse from comment #32)
> As I mentioned on IRC, I do not see "browser.onboarding.enabled" as false,
> which I have set in the files. I believe I checked what bookmark I was on,
> and also verified that the files had it set as false. @whimboo mentioned
> about using unpatched Marionette, but I do not understand how to do that.

As noted in my review there is a spelling mistake which causes the appropriate pref not to appear as false when testing with Marionette. Anyway, in your case you should always see the blue box at the top-left corner when opening a new tab.
Assignee: nobody → akk5597
Status: NEW → ASSIGNED
Flags: needinfo?(hskupin)
Aditya, are you going to finish off this patch? Please let us know. Thanks.
Flags: needinfo?(akk5597)
I'm extremely sorry but I have been busy with my college and was unable to continue working on this. I'll need some time to get back to this and will try to upload a patch within a few hours. Sorry for the inconvenience.
Flags: needinfo?(akk5597)
Comment on attachment 8983511 [details]
Bug 1443572 - Disable Photon onboarding experience for Marionette and geckodriver

https://reviewboard.mozilla.org/r/249364/#review269400

::: testing/geckodriver/src/prefs.rs:11
(Diff revision 3)
>  // channel builds of Firefox.  Removing a preference from this file
>  // will cause regressions, so please be careful and get review from
>  // a Testing :: Marionette peer before you make any changes to this file.
>  
>  lazy_static! {
> -    pub static ref DEFAULT: [(&'static str, Pref); 52] = [
> +    pub static ref DEFAULT: [(&'static str, Pref); 53] = [

This change cannot be applied anymore because your local checkout of mozilla-central is too old. Please run a pull -u on central, and fix this merge conflict.

::: testing/marionette/client/marionette_driver/geckoinstance.py:518
(Diff revision 3)
>  
>          # Do not show the EULA notification which can interfer with tests
>          "browser.EULA.override": True,
>  
> +        # Disable Photon onboarding experience
> +        "broswer.onboarding.enabled": False,

As mentioned already last time there is a spelling error. That's why onboarding is not getting disabled.
Attachment #8983511 - Flags: review?(hskupin) → review-
It looks like that in recent versions of Nightly the notification isn't shown anymore. Instead there is an indication with a small blue dot on the Nightly logo of `about:newtab`. Anyway, I think it's still worth fixing it.
(In reply to Henrik Skupin (:whimboo) from comment #39)
> It looks like that in recent versions of Nightly the notification isn't
> shown anymore. Instead there is an indication with a small blue dot on the
> Nightly logo of `about:newtab`. Anyway, I think it's still worth fixing it.

Then it should be added with a comment saying when it can be removed.
(In reply to Andreas Tolfsen 「:ato」 from comment #40)
> Then it should be added with a comment saying when it can be removed.

No, we should keep it until it completely got removed. Also as for any guided tour this can change again over time.
Assignee: akk5597 → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.