1417155 - Add new Home / New Tab page preference section for default / blank / custom / add-on

Status: RESOLVED FIXED

(Firefox :: New Tab Page, enhancement, P1)

Description

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

At this moment, Firefox for desktop does not have a preference for new tab page. I suggest to add an option to General preferences to use the homepage as a new tab page too. Basically the same option as is in Firefox for Android.

Comment 1

[Andrei Oprea [:andreio]]

What would that pref do? With Activity Stream enabled about:home and about:newtab provide the same experience.

There are plans to migrate Activity Stream preferences from the gear icon on the page to the main Firefox settings page.

Comment 2

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

In Firefox for Android there is an option to change the homepage to custom URL plus an extra option to use this URL for new tabs too. In Firefox for desktop there is only the option for homepage, but none to use the custom URL for new tabs too. I think it makes sense to introduce this option especially after the about:home and about:newtab have been unified. Now if the user changes the homepage, it will actually apply to the homepage opened for new Firefox instance (about:home), but not for new tabs (about:newtab).

Comment 3

[Tim Spurway [:tspurway]]

One way to get this functionality is to use a Web Extension that resets the about:newtab page to the URL.

Comment 4

[Ed Lee :Mardak]

uiwanted: More details on how the new grouped section looks / behaves

Comment 5

[Ed Lee :Mardak]

More uiwanted: Should the page controls be always visible or behind radio buttons:

E.g.,

Home Page
[ default / custom / add-on ▼ ]

New Tab
[ default / custom / add-on ▼ ]


vs

Home and New Tab Pages
(o) Use default for both home and new tab
( ) Use a custom url for both home and new tab
( ) Show a blank page for both home and new tab
( ) Use a different page for each home and new tab

Where selecting the last radio option would expand to the above?

Comment 6

[Ed Lee :Mardak]

Sounds like the latest update is to keep the home page functionality but move things into drop downs:

Home page
[ Firefox Home / Blank Page / Use Current Pages / Use Bookmark… / Custom ▼ ]
[ text box shows up when Custom is selected ]

Where selecting "use current" or "use bookmark" will end up making the selection switch to Custom. Use Current Pages will fill in the text box with open tabs. Use Bookmark… will have the dialog that will cause the text box to fill in. Custom by itself will show an empty text box. I guess if the text box is left blank, it'll revert to Firefox Home. ?


Then for new tabs:

New Tab page
[ Firefox Home / Blank Page / Use Bookmark… / Custom  ▼ ]
[ text box shows up when Custom is selected ]

Similar to above but not "Use Current Pages". This means we'll need to reintroduce a pref to set the new tab page. Perhaps to satisfy bug 1383599, selecting Blank Page will set browser.newtabpage.enabled to false. ?

Comment 7

[Florian Quèze [:florian]]

(In reply to Ed Lee :Mardak from comment #6)

> This means we'll need to
> reintroduce a pref to set the new tab page.

The newtab page pref was removed because it was actively abused by search hijackers (see bug 1118285). Do you have plans to avoid this re-occurring?

Comment 8

[Ed Lee :Mardak]

I remember seeing bug 1426438 go by for removing NewTabURL, but I guess I thought the original bug removing the pref was from add-on abuse. If it's a problem to have any custom new tab page as a pref, then I guess the new tab options would just be [ Firefox Home / Blank Page ▼ ]? Do webextensions allow for setting a url statically or even dynamically via some event (although this latter one would be harder to display in about:preferences)?

Comment 9

[Ed Lee :Mardak]

uifeedback: https://mozilla.invisionapp.com/share/EGFI16P8Y36#/screens/276669529_Preferences_-_Home_Selected

Use Home icon for a "Home" section in about:preferences (might be slightly different from the toolbar icon)

Show sub section with dropdowns for "home & new windows" and "new tabs":
home maintains existing functionality but via dropdown items that will reveal custom input box as in comment 6
new tab only has "Firefox Home (Default)" and blank.. and optionally extension if installed (like existing behavior)

not for this bug: General section should have a checkbox to "restore last session" instead of the 3 radio of home vs blank vs restore. as home can be set to blank now.

not for this bug: there's a restore defaults section

Comment 10

[Ed Lee :Mardak]

(In reply to Ed Lee :Mardak from comment #9)
> https://mozilla.invisionapp.com/share/EGFI16P8Y36#/screens/276669529_Preferences_-_Home_Selected
designakt, how should the incoming dropdown changes handle webextensions? They should show up as drop down items? Can there be multiple webextensions to choose from? What happens when switching to "Firefox Home" or Blank?

Or if we don't change the behavior now for webextension, I would guess the dropdown gets disabled and shows that a webextension is controlling home and/or new tab pages?

Comment 11

[Markus Jaritz [:designakt] (UX)]

Attachment: New_NewTabOverride.png

(In reply to Ed Lee :Mardak from comment #10)
> (In reply to Ed Lee :Mardak from comment #9)
> > https://mozilla.invisionapp.com/share/EGFI16P8Y36#/screens/276669529_Preferences_-_Home_Selected

> designakt, how should the incoming dropdown changes handle webextensions?
Please add the extension to the dropdown and use the current extension notice below the relevant section (if the extension is selected) - so that people can disable the extension.

> They should show up as drop down items?
Yes, please.

> Can there be multiple webextensions to choose from?
Yes... we currently let the last installed control the page... but this dropdown could expose previously installed extensions... if they are enabled.

> What happens when switching to "Firefox Home" or Blank?
The extension notice disapears. (The extension stays enabled, but is not controling the page anymore.)

> Or if we don't change the behavior now for webextension, I would guess the
> dropdown gets disabled and shows that a webextension is controlling home
> and/or new tab pages?
That might be a fallback, but seams less elegant.

Bob, is the proposed behaviour possible? Or do we have to disable the extension for the user to be able to change what page to show.

Comment 12

[Bob Silverberg [:bsilverberg]]

(In reply to Markus Jaritz [:designakt] (UX) from comment #11)
> Created attachment 8948938 [details]
> New_NewTabOverride.png
> 
> (In reply to Ed Lee :Mardak from comment #10)
> > (In reply to Ed Lee :Mardak from comment #9)
> > > https://mozilla.invisionapp.com/share/EGFI16P8Y36#/screens/276669529_Preferences_-_Home_Selected
> 
> > designakt, how should the incoming dropdown changes handle webextensions?
> Please add the extension to the dropdown and use the current extension
> notice below the relevant section (if the extension is selected) - so that
> people can disable the extension.
> 
> > They should show up as drop down items?
> Yes, please.
> 
> > Can there be multiple webextensions to choose from?
> Yes... we currently let the last installed control the page... but this
> dropdown could expose previously installed extensions... if they are enabled.
> 
> > What happens when switching to "Firefox Home" or Blank?
> The extension notice disapears. (The extension stays enabled, but is not
> controling the page anymore.)
> 
> > Or if we don't change the behavior now for webextension, I would guess the
> > dropdown gets disabled and shows that a webextension is controlling home
> > and/or new tab pages?
> That might be a fallback, but seams less elegant.
> 
> Bob, is the proposed behaviour possible? Or do we have to disable the
> extension for the user to be able to change what page to show.

It's certainly possible. We'd have to make some changes to the way precedence is handled. Right now it's all based on install date, but this would require us to give a different extension precedence regardless of its install date. That's doable, and I agree that this interface would be better, but we just have to realize that it's not an insignificant amount of work.

If we go that route, we probably don't even need the "Disable" button beside the extension controlled notice, and maybe we don't need the notice at all, if we think the presence of an extension in the drop down will be enough to draw the user's attention to the fact that an extension is controlling the setting.

Comment 13

[Markus Jaritz [:designakt] (UX)]

(In reply to Bob Silverberg [:bsilverberg] from comment #12)
> If we go that route, we probably don't even need the "Disable" button beside
> the extension controlled notice, and maybe we don't need the notice at all,
> if we think the presence of an extension in the drop down will be enough to
> draw the user's attention to the fact that an extension is controlling the
> setting.

I've been thinking about that too. I opted for the notice to make it clear that this is an extension, and to give users a chance to disable the extension. (And with that align with the pattern we established)

Comment 14

[Ed Lee :Mardak]

flod, similar to your concern from bug 1404890 comment 12 of strings late in nightly, the strings for this particular bug's changes are most likely landing directly in m-c as that's where they exist. Are there timing concerns there too or there's regular mozilla-beta l10n updates given the lack of aurora localization period. (Sorry, it's been a while since we landed strings in m-c.)

Comment 15

[Francesco Lodolo [:flod]]

(In reply to Ed Lee :Mardak from comment #14)
> flod, similar to your concern from bug 1404890 comment 12 of strings late in
> nightly, the strings for this particular bug's changes are most likely
> landing directly in m-c as that's where they exist. Are there timing
> concerns there too or there's regular mozilla-beta l10n updates given the
> lack of aurora localization period. (Sorry, it's been a while since we
> landed strings in m-c.)

OK, I assumed they were AS strings, not just normal Firefox strings in /preferences.

The deadline to land strings for 60 is merge day (March 12), but you really want them to land before that week-end to be safe.

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

:Mardak - if you're aiming to land those strings in Firefox (m-c) and not inject them dynamically from AS addon, could you use Fluent? We're migrating Preferences to Fluent right now (tracking bug 1415730) - landed preferences.xul switch already, and will switch main.xul next week.

If you'll land it as .properties, we'll want to migrate them to .ftl anyway soon.

Comment 17

[Ed Lee :Mardak]

Sure, I'll keep an eye on those other about:preferences fluent migration bugs / patches to see how to do it for this bug.

Comment 18

[Ed Lee :Mardak]

There's updated designs for the extension stuff:
https://mozilla.invisionapp.com/share/EGFI16P8Y36#/screens

In particular each window/tab dropdown has a separator before listing all extensions. And there's 3 possible messages showing "An extension is controlling the page you see on (New Windows and Home|New Tabs|New Windows, New Tabs and Home). [Manage Extension]"

(In reply to Bob Silverberg [:bsilverberg] from comment #12)
> I agree that this interface would be better, but we just have to realize
> that it's not an insignificant amount of work.
It's desired to have these designs implemented for 60, which is just a few weeks left. What needs to be updated on the webextension management side for both controlling new tab and new windows/home?

Alternatively, if it can't be done for 60, we'll need to figure out designs for combining the existing extension behavior, which I would guess looks like:

Home
Open Home and New Windows with: [an extension]
An extension, <name>, is controlling your home page. [Disable Extension]
Open New Tabs with: [an extension]
An extension, <name>, is controlling your New Tab page. [Disable Extension]

Where the "An extension, <name>…" is existing code/ui but the dropdown needs to have a new entry. Where I guess switching away from "an extension" is equivalent to clicking [Disable extension] (for all extensions that would have controlled??)

Alternatively, maybe the dropdown gets disabled if an extension is controlling to avoid additional oddness of multiple attempting to control?

Comment 19

[Bob Silverberg [:bsilverberg]]

Thanks for flagging me on this, Ed. I don't believe that I will be the one working on it, however, so I am going to needinfo Mike Conca who can help with figuring out who can work on this and if it's possible to get it done for 60.

Comment 20

[Mike Conca [:mconca]]

Request received.  Talking to ddurst to figure this out.

Comment 21

[Mike Conca [:mconca]]

:ddurst and I talked. He needs to speak with a couple of people on his team before finalizing the path forward.  Dropping a NI here for that.

Comment 22

[Markus Jaritz [:designakt] (UX)]

I talked with Aaron about his new design for the new Tab / Home override message for extensions. 
We noticed some nesecary adjustments to his design to ensure the behavior for extensions stays consistent:
See extension management section in: https://mozilla.invisionapp.com/share/EGFI16P8Y36

When updating the apperance of the new tab / home message, we also need to update the style of the other overrides. 
Those are: Tracking Protection, Containers, Cookies and Proxy.

As users now can change the homepage / new Tab setting through the drop-down even when an extension is in control, we need to ensure that this is translated into disabeling the extension - as is the behavior with the current message. And re-enable it when selecting it from the drop-down again.

And we had to add a 4th option for the extension message, for the case new Tab and Home are controlled by 2 different extensions.

I'll needinfo bob to ensure those changes are feasible.

Comment 23

[Bob Silverberg [:bsilverberg]]

I'm probably not the best person to be investigating/discussing this. I think Mark would be more appropriate so I'm going to transfer my needinfo to him.

Comment 24

[Mark Striemer [:mstriemer]]

As discussed in our meeting today I'm not sure we want to disable an extension if the user changes their choice. The extension might be providing some other functionality as well such as adding a search engine, awesome bar keyword, page action, context menu action, etc.

Looking at the mocks [1] for the new Home section in Preferences, I don't see how this accomplishes what was requested in comment 0. If I have my homepage set to https://blog.mozilla.org and the New Tab is set to "Firefox Home" wouldn't I still see about:newtab on new tabs?

With regards to webextensions for New Tab and homepage in 60: it should be possible to add the dropdown and disable it when an extension is controlling the setting, similar to how the homepage section works. I can take a look adding this or providing support.

To avoid disabling this option, as Bob mentioned, this would be a decent amount of work on the webextensions side since it moves away from the last installed wins model into a user choice model. This seems like a much better solution, so I'm happy to work on fixing this in a separate bug for a later release.

[1] https://mozilla.invisionapp.com/share/EGFI16P8Y36

Comment 25

[Ed Lee :Mardak]

tspurway, it looks like it would be a bit tricky to add experimentation to the mozilla-central code controlling about:preferences content, and if we scope this bug to just creating a "Home" sidebar/panel to contain the existing "Home page" settings, then during a experiment where we turn off activity stream about:preferences, the "Home" panel would only have the "Home page" settings, and that would probably be a bit strange. Alternatively, if we don't fix this bug for 60, i.e., leave the existing "Home page" settings and adding activity stream section prefs under that, the General panel would be a little bit longer than what it is now.

So it seems like if we do want to run an experiment in 60 beta with activity stream prefs in-page sidebar vs about:preferences, we probably don't want to fix this bug for 60?

Comment 26

[Kate Hudson :k88hudson]

We won't get these additional fields in for 60, but we're going to try to land the new prefs section (with just the Firefox Home Content part) since Bryan and Aaron have indicated it's essential for the design.

Comment 27

[Hlsg]

(In reply to Kate Hudson :k88hudson from comment #26)
> We won't get these additional fields in for 60, but we're going to try to
> land the new prefs section (with just the Firefox Home Content part) since
> Bryan and Aaron have indicated it's essential for the design.

How about the underlying about:config prefs? Any chance of seeing those in 60? Why browser.newtab.url was completely removed instead of being restricted to about:blank, about:newtab and about:home is beyond my understanding of these things.

Comment 28

[Kate Hudson :k88hudson]

Attachment: Bug 1417155 - Add Home section to about:preferences for Activity Stream preferences

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

Comment 29

[Kate Hudson :k88hudson]

Attachment: Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

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

Comment 30

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/1-2/

Comment 31

[Kate Hudson :k88hudson]

Ok, I've updated patch with the following changes:

- Adds the #home section to about:preferences
- Adds the New Windows and Tabs section with options for changing the homepage and new tab
- Ports functionality for disabling the appropriate UI when homepage/new tab are locked via prefs or overridden by extensions

Still to do (maybe in a follow-up patch, since this is already pretty big)
- Show the web extension in the drop down as per specs rather than

Comment 32

[Kate Hudson :k88hudson]

... show a message below
- Change the startup preference to be a checkbox rather than radio buttons

Comment 33

[Ed Lee :Mardak]

(In reply to Kate Hudson :k88hudson from comment #32)
> - Change the startup preference to be a checkbox rather than radio buttons
Already split to a followup ;) bug 1440219

Comment 34

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/2-3/

Comment 35

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

https://reviewboard.mozilla.org/r/227472/#review233618

::: browser/locales/en-US/browser/preferences/preferences.ftl:341
(Diff revision 3)
>      .label = Settings…
>      .accesskey = e
> +
> +## Home Section
> +
> +new-windows-tabs-header = New Windows and Tabs

can you use a prefix here? We try to keep each section to use their own prefix (the top part of the file is for chrome of prefs, and has no section).

For example, starting each string with `home-` would work!

Comment 36

[Francesco Lodolo [:flod]]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

https://reviewboard.mozilla.org/r/227472/#review233616

::: browser/locales/en-US/browser/preferences/preferences.ftl:343
(Diff revision 2)
> +
> +## Home Section
> +
> +new-windows-tabs-header = New Windows and Tabs
> +
> +new-windows-tabs-description = Chose what you see when you open your homepage, new windows, and new tabs

typo: Choose

::: browser/locales/en-US/browser/preferences/preferences.ftl:347
(Diff revision 2)
> +
> +new-windows-tabs-description = Chose what you see when you open your homepage, new windows, and new tabs
> +
> +## Home Section - Home Page Customization
> +
> +homepage-mode-label = Homepage and new windows:

We removed almost all colons from prefs. Are these in line with the existing prefs?

::: browser/locales/en-US/browser/preferences/preferences.ftl:353
(Diff revision 2)
> +
> +restore-defaults =
> +    .label = Restore Defaults
> +    .accesskey = R
> +
> +homepage-mode-choice-default

Please add a note that this can be translated, but Firefox needs to be treated as a brand and kept in English.

::: browser/locales/en-US/browser/preferences/preferences.ftl:357
(Diff revision 2)
> +
> +homepage-mode-choice-default
> +    .label = Firefox Home (Default)
> +
> +homepage-mode-choice-custom
> +    .label = Custom URL(s)...

Please use … (single Unicode character), not 3 dots.

::: browser/locales/en-US/browser/preferences/preferences.ftl:362
(Diff revision 2)
> +    .label = Custom URL(s)...
> +
> +homepage-mode-choice-blank
> +    .label = Blank Page
> +
> +newtabs-mode-label = New tabs:

Same question here about having colon

Comment 37

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #35)
> can you use a prefix here? We try to keep each section to use their own
> prefix (the top part of the file is for chrome of prefs, and has no section).
> 
> For example, starting each string with `home-` would work!

Oh, I realize now that my request is not that trivial.

For strings you want to preserve, leave them as is to avoid invalidating them.

For new strings in the Home section, please, use `home-`. The subsection is small enough that can be part of `home-` naming scheme.

So:

`home-new-windows-tabs-header`
`home-new-windows-tabs-description`
`home-mode-label`
`home-restore-defaults`
`home-mode-choice-default`
`home-mode-choice-custom`
`home-mode-choice-blank`
`home-newtabs-mode-label`

(and preserve:)
`use-current-pages`
`choose-bookmark`

You can also use the longer `homepage-` if you prefer! :)

thnx!

Comment 38

[Code Review Bot [:reviewbot]]

Comment on attachment 8958572 [details]
Bug 1417155 - Add Home section to about:preferences for Activity Stream preferences

https://reviewboard.mozilla.org/r/227470/#review233680


Code analysis found 1 defect in this patch:
 - 1 defect 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


::: browser/components/preferences/in-content/preferences.js:55
(Diff revision 1)
>  function init_all() {
>    Preferences.forceEnableInstantApply();
>  
>    gSubDialog.init();
>    register_module("paneGeneral", gMainPane);
> +  register_module("paneHome", gHomePane);

Error: 'ghomepane' is not defined. [eslint: no-undef]

Comment 39

[Kate Hudson :k88hudson]

Comment on attachment 8958572 [details]
Bug 1417155 - Add Home section to about:preferences for Activity Stream preferences

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227470/diff/1-2/

Comment 40

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/3-4/

Comment 41

[Francesco Lodolo [:flod]]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

https://reviewboard.mozilla.org/r/227472/#review233970

::: browser/locales/en-US/browser/preferences/preferences.ftl:355
(Diff revisions 3 - 4)
>      .label = Restore Defaults
>      .accesskey = R
>  
> -homepage-mode-choice-default
> +# "Firefox" should be treated as a brand and kept in English, while "Home"
> +# should be localized matching the title of the #home panel above.
> +home-mode-choice-default

You need an equal sign here after the ID, even when there's no value (see home-restore-defaults for example).

Same applies to the next 2 strings.

Comment 42

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/4-5/

Comment 43

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8958572 [details]
Bug 1417155 - Add Home section to about:preferences for Activity Stream preferences

https://reviewboard.mozilla.org/r/227470/#review233990

lgtm!

Comment 44

[Kate Hudson :k88hudson]

Michelle, would you be OK with dropping the colons on the labels for the New Windows and Tabs section (i.e. the strings "Homepage and new windows" as well as "New tabs")?

flod pointed out that some work was done to remove colons from the other areas of about:preferences (see: https://bugzilla.mozilla.org/show_bug.cgi?id=1382135 and https://bugzilla.mozilla.org/show_bug.cgi?id=1399699)

Comment 45

[Francesco Lodolo [:flod]]

Comment on attachment 8958572 [details]
Bug 1417155 - Add Home section to about:preferences for Activity Stream preferences

https://reviewboard.mozilla.org/r/227470/#review233992

Comment 46

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/5-6/

Comment 47

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/6-7/

Comment 48

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

https://reviewboard.mozilla.org/r/227472/#review233616

> We removed almost all colons from prefs. Are these in line with the existing prefs?

Ok, I talked to Michelle and she agreed they should be removed

Comment 49

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/7-8/

Comment 50

[Francesco Lodolo [:flod]]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

https://reviewboard.mozilla.org/r/227472/#review234388

A couple of minor nits, but the FTL itself looks good. Leaving the code to more suitable eyes.

::: browser/locales/en-US/browser/preferences/preferences.ftl:347
(Diff revision 8)
> +
> +home-new-windows-tabs-description = Choose what you see when you open your homepage, new windows, and new tabs
> +
> +## Home Section - Home Page Customization
> +
> +home-mode-label = Homepage and new windows

Maybe "home-homepage-mode-label" for consistency? I would also move "home-newtabs-mode-label" close to this string

::: browser/locales/en-US/browser/preferences/preferences.ftl:354
(Diff revision 8)
> +home-restore-defaults =
> +    .label = Restore Defaults
> +    .accesskey = R
> +
> +# "Firefox" should be treated as a brand and kept in English, while "Home"
> +# should be localized matching the title of the #home panel above.

Let's just say

# "Firefox" should be treated as a brand and kept in English, while "Home" and "(Default)" can be localized.

Reasons:
* Localizers are exposed to a string at a time, they have no concept of above looking at the file.
* I don't think they necessarily need to use the same words as the sidebar (different context, different space constraints).

Comment 51

[Francesco Lodolo [:flod]]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

https://reviewboard.mozilla.org/r/227472/#review234390

::: browser/components/preferences/in-content/home.xul:55
(Diff revision 8)
> +          <textbox id="homePageUrl"
> +                  class="uri-element check-home-page-controlled"
> +                  data-preference-related="browser.startup.homepage"
> +                  type="autocomplete"
> +                  autocompletesearch="unifiedcomplete"
> +                  placeholder="Paste a URL..." />

This one is hard-coded, and should use a single unicode character for the ellipsis

Comment 52

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/8-9/

Comment 53

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

Comment on attachment 8958572 [details]
Bug 1417155 - Add Home section to about:preferences for Activity Stream preferences

https://reviewboard.mozilla.org/r/227470/#review234776

::: browser/components/preferences/in-content/preferences.xul:151
(Diff revision 2)
> +                      helpTopic="prefs-home"
> +                      data-l10n-id="category-home"
> +                      data-l10n-attrs="tooltiptext"
> +                      align="center"
> +                      hidden="true">
> +

nit, please remove this blank line

Comment 54

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

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

https://reviewboard.mozilla.org/r/227472/#review234778

Some of the requests below are pertaining to code that was there before your changes but since you're moving it now would be a good time to fix them.

::: browser/components/enterprisepolicies/tests/browser/browser_policy_set_homepage.js:168
(Diff revision 9)
> +    is(content.document.getElementById("homeMode").disabled, true,
> +      "Homepage menulist should be disabled");
> +    is(content.document.getElementById("homePageUrl").disabled, true,
> +      "Homepage custom input should be disabled");
> +    is(content.document.getElementById("useCurrentBtn").disabled, true,
> -       "\"Use Current Page\" button should be disabled");
> +        "\"Use Current Page\" button should be disabled");

These lines got an extra space added at the beginning but they shouldn't have. They should start at the same column as the 'c' in `content`.

::: browser/components/preferences/in-content/home.js:60
(Diff revision 9)
> +   * _renderCustomSettings: Hides or shows the UI for setting a custom
> +   * homepage URL
> +   * @param {bool} shouldShow Should the custom UI be shown?
> +   */
> +  _renderCustomSettings(shouldShow) {
> +    const el = document.getElementById("customSettings");

Please rename `el` here to be more specific since later in the function it can be confusing which element `el` is referring to.

::: browser/components/preferences/in-content/home.js:96
(Diff revision 9)
> +  /**
> +   * _isNotAboutPreferences: Is a given tab set to about:preferences?
> +   * @param {Element} aElement A tab
> +   * @returns {bool} Is the linkedBrowser of aElement set to about:preferences?
> +   */
> +  _isNotAboutPreferences(aElement) {

Functions (and variables) should be written in the affirmative, otherwise adding a negation to them can become confusing.

Please change this function to _isTabAboutPreferences()

Note too that the function expects a Tab, not a generic `element` so the argument name should be changed too.

::: browser/components/preferences/in-content/home.js:109
(Diff revision 9)
> +  _getTabsForHomePage() {
> +    let tabs = [];
> +    let win = Services.wm.getMostRecentWindow("navigator:browser");
> +
> +    if (win && win.document.documentElement
> +      .getAttribute("windowtype") == "navigator:browser") {

indentation here is broken

::: browser/components/preferences/in-content/home.js:113
(Diff revision 9)
> +    if (win && win.document.documentElement
> +      .getAttribute("windowtype") == "navigator:browser") {
> +      // We should only include visible & non-pinned tabs
> +
> +      tabs = win.gBrowser.visibleTabs.slice(win.gBrowser._numPinnedTabs);
> +      tabs = tabs.filter(this._isNotAboutPreferences);

tabs = tabs.filter(tab => !this._isTabAboutPreferences(tab));

::: browser/components/preferences/in-content/home.js:136
(Diff revision 9)
> +      el.value = this.HOME_MODE_CUSTOM;
> +    }
> +  },
> +
> +  _setInputDisabledStates(isControlled) {
> +

please remove this blank line

::: browser/components/preferences/in-content/home.js:186
(Diff revision 9)
> +    switch (value) {
> +      case this.HOME_MODE_BLANK:
> +        return false;
> +      default:
> +        return true;

This can be simplified to just:
`return value != this.HOME_MODE_BLANK;`

::: browser/components/preferences/in-content/home.js:225
(Diff revision 9)
> +   * forms.
> +   */
> +  async _updateUseCurrentButton() {
> +    let useCurrent = document.getElementById("useCurrentBtn");
> +    let tabs = this._getTabsForHomePage();
> +

please remove this blank line

::: browser/components/preferences/in-content/home.js:227
(Diff revision 9)
> +  async _updateUseCurrentButton() {
> +    let useCurrent = document.getElementById("useCurrentBtn");
> +    let tabs = this._getTabsForHomePage();
> +
> +    const tabCount = tabs.length;
> +

please remove this blank line

::: browser/components/preferences/in-content/home.js:244
(Diff revision 9)
> +   * Sets the home page to the current displayed page (or frontmost tab, if the
> +   * most recent browser window contains multiple tabs), updating preference
> +   * window UI to reflect this.

This comment seems a bit wrong, since as I read it it sounds like it will set a singular home page, but it will actually set the home page to the collection of non-about:preferences pages that are opened.

::: browser/components/preferences/in-content/main.js
(Diff revision 9)
> -   * browser.startup.page
> -   * - what page(s) to show when the user starts the application, as an integer:
> -   *
> -   *     0: a blank page
> -   *     1: the home page (as set by the browser.startup.homepage pref)
> -   *     2: the last page the user visited (DEPRECATED)
> -   *     3: windows and tabs from the last session (a.k.a. session restore)
> -   *
> -   *   The deprecated option is not exposed in UI; however, if the user has it
> -   *   selected and doesn't change the UI for this preference, the deprecated
> -   *   option is preserved.

We should keep this documentation in tree. It's getting removed here but I don't see it being added anywhere.

Comment 55

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

https://reviewboard.mozilla.org/r/227472/#review234806

Comment 56

[Kate Hudson :k88hudson]

Comment on attachment 8958572 [details]
Bug 1417155 - Add Home section to about:preferences for Activity Stream preferences

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227470/diff/2-3/

Comment 57

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/9-10/

Comment 58

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s a8ce37674daafc23dbe9d9e4203039c5feee48b8 -d 87a99640fb81: rebasing 452944:a8ce37674daa "Bug 1417155 - Add Home section to about:preferences for Activity Stream preferences r=flod,gandalf,jaws"
merging browser/components/preferences/in-content/preferences.js
merging browser/components/preferences/in-content/preferences.xul
merging browser/locales/en-US/browser/preferences/preferences.ftl
rebasing 452945:0ab7c8c40b23 "Bug 1417155 - Add new Home/New Tab page section to about:prefs#home r=flod,gandalf,jaws" (tip)
merging browser/components/enterprisepolicies/tests/browser/browser_policy_set_homepage.js
merging browser/components/preferences/in-content/main.js
merging browser/components/preferences/in-content/main.xul
merging browser/components/preferences/in-content/preferences.xul
merging browser/components/preferences/in-content/tests/browser_extension_controlled.js
merging browser/components/preferences/in-content/tests/browser_homepages_filter_aboutpreferences.js
merging browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_1.js
merging browser/components/preferences/in-content/tests/browser_search_within_preferences_1.js
merging browser/locales/en-US/browser/preferences/preferences.ftl
warning: conflicts while merging browser/components/preferences/in-content/tests/browser_extension_controlled.js! (edit, then use 'hg resolve --mark')
warning: conflicts while merging browser/components/preferences/in-content/tests/browser_homepages_filter_aboutpreferences.js! (edit, then use 'hg resolve --mark')
warning: conflicts while merging browser/locales/en-US/browser/preferences/preferences.ftl! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 59

[Kate Hudson :k88hudson]

Comment on attachment 8958572 [details]
Bug 1417155 - Add Home section to about:preferences for Activity Stream preferences

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227470/diff/3-4/

Comment 60

[Kate Hudson :k88hudson]

Comment on attachment 8958573 [details]
Bug 1417155 - Add new Home/New Tab page section to about:prefs#home

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/227472/diff/10-11/

Comment 61

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7852878194fc
Add Home section to about:preferences for Activity Stream preferences r=flod,gandalf,jaws
https://hg.mozilla.org/integration/autoland/rev/810204c29a4d
Add new Home/New Tab page section to about:prefs#home r=flod,gandalf,jaws

Comment 62

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/7852878194fc
https://hg.mozilla.org/mozilla-central/rev/810204c29a4d

Comment 65

[Pascal Chevrel:pascalc]

Kate, this is a UI change for the end-user that should be added to release notes, could you nominate it for addition please? (in the Firefox tracking flags, set relnote-firefox to ?, click on the "comment" link and fill in the information).

Thanks

Comment 66

[Pascal Chevrel:pascalc]

I forgot to add the NI in comment #65

Comment 67

[Kate Hudson :k88hudson]

Release Note Request (optional, but appreciated)

[Why is this notable]:
Adds a new about:preferences section

[Affects Firefox for Android]:
Firefox

[Suggested wording]:
The settings for customizing your homepage and new tab page in Firefox have been moved to a new Preferences section, Home. It can be accessed from the sidebar in Preferences or by visiting about:preferences#home.

[Links (documentation, blog post, etc)]:
None

Comment 68

[Pascal Chevrel:pascalc]

(In reply to Kate Hudson :k88hudson from comment #67)
> Release Note Request (optional, but appreciated)
> 
> [Why is this notable]:
> Adds a new about:preferences section
> 
> [Affects Firefox for Android]:
> Firefox
> 
> [Suggested wording]:
> The settings for customizing your homepage and new tab page in Firefox have
> been moved to a new Preferences section, Home. It can be accessed from the
> sidebar in Preferences or by visiting about:preferences#home.
> 
> [Links (documentation, blog post, etc)]:
> None

Thank you, this was added to Nightly release notes for 61. For beta and final release notes, our release managers will decide on the potential inclusion and final wording, hence keeping the flag as 'relnote ?'.