1498332 - Redesign and rearrange Thunderbird Preferences/Options similar to Firefox

Status: RESOLVED FIXED

(Thunderbird :: Preferences, enhancement)

Description

[Elio Qoshi [:Elio]]

In the first UX Team Meeting we decided that it would be a good first thing to tackle the current Preferences / Options and Account Settings screens to be unified and displayed in a similar manner as it happens on Firefox. This option is in fact already available in Thunderbird but needs to be manually enabled by going to the Config editor and enable mail.preferences.inContent

There is a bug in Lightning which actually prevents us to enable this by default however: #1096006

We should work on unifying the menus into Preferences / Options and create mockups for these views first.

Comment 1

[Jorg K (CEST = GMT+2)]

Hmm, you should keep up with the flow. The stand-alone options window has been removed in TB 63 already, see release notes:
https://www.thunderbird.net/en-US/thunderbird/63.0beta/releasenotes/
The option you're quoting no longer exists.

As for accounts in a tab (bug 1096006 and bug 1202480): That may not happen at all, instead we might port this straight to HTML, see bug 1468167 with attachment 8985174 [details], attachment 8985174 [details]).

P.S.: Please do not write #1096006, instead write bug 1096006 so hovering works.

Comment 2

[Elio Qoshi [:Elio]]

Don't kill the messenger :)

This bug resulted out of the last UX meeting whereas Richard Marti gave us additional context. I'd suggest us all to get on the same page on these as we will otherwise end up with different people proposing different directions. @Ryan @Richard, I'd appreciate your input here.

Comment 3

[Elio Qoshi [:Elio]]

Also notes from the meeting by Ryan: https://docs.google.com/document/d/1CQPxquEpyUa-B65s37IlT2z7tkVM1pb6d5UTOPuqMaA/edit

Comment 4

[Richard Marti (:Paenglab)]

First focus only on the prefs. Account manager can be done later.

Comment 5

[Ryan Lee Sipes]

Yes, that note about how to enable it is for new UX contributors to be able to take a look at what this looks like. This bug will serve as a location to share mock-ups, sketches, and ideas for how we might improve the layout and experience of Preferences and Account Settings now that they are being moved into tabs.

Those links are helpful to see how this currently looks with attachment 8985174 [details] - and the links to the other bugs around account settings. This has to be done and is pretty low-hanging fruit, so this is a good place for the UX Action Committee to spin its cycles.

Comment 6

[Elio Qoshi [:Elio]]

So we have some updates on this since we got the green light to move forward.

Currently, with `mail.preferences.inContent` enabled, we have 25 separate views in the Preferences of Thunderbird (Categories+Tabs), making it quite difficult to find's needed as quite various things are in different places and there is a lot of clicking involved.

The categorization is also problematic right now since every Category has a General and Advanced tab, while there is a General and Advanced tab separately as well. We went with suggesting more specific wording and incorporate those options under categories which would explain the nature of these options better than simply "Advanced" for example.

Furthermore, copy wise there are some problems with the 1st and 2nd person in the Interface. Both are used in different scenarious such as:

--

Thunderbird can analyze messages for suspected email scams by looking for common techniques used to deceive you.

[ ] Tell me if the message I'm reading is a suspected email scam

--

It's suggested to consider a UI as a conversation between the interface and the user, so we should always go with the 2nd person. This is also aligned with the Firefox style guide and copywriting. We addressed this in our proposal as well.

You can find a prototype of this design here:

https://www.figma.com/proto/DAIuhXdQ0G9AExbKLCQZ1HSE/Settings?node-id=12%3A560&viewport=288%2C524%2C0.426706&scaling=min-zoom

The categories on the sidebar are clickable and all 25 views have been distilled into 5 views here as well, grouping up similar options next to each other.

We will be writing a Design Study soon to provide statistics and best UX practices to explain those design decisions better as there are quite a few changes in the prototype we worked on. In the meantime we are also scheduling some user testing sessions to test both prototype and current Thunderbird Preferences ( with `mail.preferences.inContent` enabled ) and summarize those results. 

Notes:
- We used the Thunderbird style guide for the components in the prototype and the Ubuntu font. Styling of these change depending on the OS you use Thunderbird on of course.
- Feedback appreciated on the overall structure, flow and logic. Have a look at how Firefox deals with the Preferences views to understand some of the motivations behind the design decisions better as well.

I hope you like the work done so far!

Comment 7

[Richard Leger]

Attachment: certificate_manager_not_adjusting_its_width_to_parent_container_width.png

FYI, currently in TB 64.0b3 (32-bit) the Certificate Manager modal windows does not adjust its width automatically to the parent container. This cause the closing button (x) and Ok button to be hidden and not available when TB is not maximised... maybe something that can be sorted out while reworking the options/settings layout UI... also seems to happen in Firefox :-)

Comment 8

[Elio Qoshi [:Elio]]

Thanks Richard, 

I'm sure this can be looked at when implementation works starts. Could you open a separate bug for that maybe to track it?

Comment 9

[Elio Qoshi [:Elio]]

Also something else: We got feedback regarding a Search field for the Preferences view similar to Firefox. If that can be implemented it would be good to have that as well. What do you think? https://discourse.opensourcedesign.net/t/vote-on-open-source-design-devroom-at-fosdem-2019/808/4

Comment 10

[Richard Leger]

(In reply to Elio Qoshi  [:Elio] from comment #8)
>
> Could you open a separate bug for that maybe to track it?

Done. Bug 1515109.

Comment 11

[Richard Leger]

(In reply to Elio Qoshi  [:Elio] from comment #9)
> Also something else: We got feedback regarding a Search field for the
> Preferences view similar to Firefox. If that can be implemented it would be
> good to have that as well. What do you think?
> https://discourse.opensourcedesign.net/t/vote-on-open-source-design-devroom-
> at-fosdem-2019/808/4

As I was at it, I created the separate Bug 1515114 for the "add search field in options" thing...

Comment 12

[Richard Leger]

I suppose this bug could be set as depending on those two bug I created to ease following up and reference to each others... is someone has the right to do so and see it as convenient feel free to do so... I don't have the power :-)

Comment 13

[Richard Leger]

May be of interest as well Bug 1512476 "Add search to the Certificates Manager"...

Comment 14

[Elio Qoshi [:Elio]]

> Also something else: We got feedback regarding a Search field for the
> Preferences view similar to Firefox. If that can be implemented it would be
> good to have that as well. What do you think?
> https://discourse.opensourcedesign.net/t/vote-on-open-source-design-devroom-
> at-fosdem-2019/808/4

I'm sorry, I linked to the wrong forum thread. This is the right one:
https://discourse.opensourcedesign.net/t/35c3-35th-chaos-communication-congress-december-27-through-30-2018-leipzig-germany/735/11

Comment 15

[Elio Qoshi [:Elio]]

Wow, the link again was again the wrong one (not sure what I did in Discourse). Here the thread on Open Source Design: https://discourse.opensourcedesign.net/t/thunderbird-preferences-views-redesign/804/3

So great news is that the Usability Study including testing of the current Preferences view and the upcoming design is completed! We are currently wrapping up things and publish the results soon.

Comment 16

[Wayne Mery (:wsmwk)]

Is this stalled for a particular reason?

Comment 17

[Ryan Lee Sipes]

AFAIK it is waiting for implementation. NEED INFOing Magnus and Alessandro so that it is on their radar.

Magnus, Alessandro - please put this on your todolist so it doesn't get forgotten.

Comment 18

[Richard Marti (:Paenglab)]

(In reply to Elio Qoshi [:Elio] from comment #15)

So great news is that the Usability Study including testing of the current Preferences view and the upcoming design is completed! We are currently wrapping up things and publish the results soon.

I can't remember seeing results.

I'm also not sure if Alessandro is happy with the Composition / Attachments / filelink presentation.

Comment 19

[Magnus Melin [:mkmelin]]

Yes stuff to do here - but not for 68.

Comment 20

[Elio Qoshi [:Elio]]

(In reply to Richard Marti (:Paenglab) from comment #18)

(In reply to Elio Qoshi [:Elio] from comment #15)

So great news is that the Usability Study including testing of the current Preferences view and the upcoming design is completed! We are currently wrapping up things and publish the results soon.

I can't remember seeing results.

I'm also not sure if Alessandro is happy with the Composition / Attachments / filelink presentation.

The results have been published in January and the discussion happened on the UX list. The study can be downloaded here: https://ura.design/projects/thunderbird

We spent a lot of time on the study and involved both community and user feedback. This not being liked comes as a surprise to me when the whole scope of work was tailored in a wider UX meeting with folks who deemed this work important and relatively low hanging. Now that Alessandro joined the core team, it would be great to discuss this and whether some design considerations are not ideal. I understand that there are other priorities now, but at the time this work started (November 2018) it was considered to be also a priority UX-wise.

Comment 21

[Alessandro Castellani [:aleca]]

Thanks for pulling me into this, I will review the study and the entire thread in order to reopen the discussion later next week.
We're on a bit of a crunch for 68 string freeze right now.
After that, I'll jump right into it.

Comment 22

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-1.patch

This implements the "General" pane according to https://www.figma.com/proto/DAIuhXdQ0G9AExbKLCQZ1HSE/Thunderbird-Preferences?node-id=12%3A560&viewport=288%2C524%2C0.426706&scaling=min-zoom&redirected=1

Aceman, Geoff, when I open the prefs I get: TypeError: can't redefine non-configurable property "Downloads"XPCOMUtils.jsm:316:19. Please can you say me how I can fix this?

Comment 23

[Geoff Lankow (:darktrojan)]

Comment on attachment 9075042 [details] [diff] [review]
1498332-rearrange-prefs-part-1.patch

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

Nice. I'm hoping you intend to have the search box too.

I think your error comes from moving downloads.js above contentAreaUtils.js. You should be able to remove the import from downloads.js (same with FileUtils).

Comment 25

[Alessandro Castellani [:aleca]]

Sweet, thanks for starting on this.
Indeed, as Geoff pointed out, we shouldn't land this until we have the Search Box implemented and usable, otherwise scrolling through the preferences will be a nightmare.

Comment 26

[Richard Marti (:Paenglab)]

Comment on attachment 9075042 [details] [diff] [review]
1498332-rearrange-prefs-part-1.patch

Geoff's hint helped me.

Comment 27

[Jorg K (CEST = GMT+2)]

(In reply to Alessandro Castellani (:aleca) from comment #25)

Sweet, thanks for starting on this.
Indeed, as Geoff pointed out, we shouldn't land this until we have the Search Box implemented and usable, otherwise scrolling through the preferences will be a nightmare.

I usually try to keep out of UI discussions, but can someone explain to me why following Firefox here is a good idea. A long time ago, FF prefs where neatly divided into categories, now they are all lumped into endless pages which require lots of scrolling, searching, etc.

Or to us the words of TB's lead UX engineer: " ... scrolling through the preferences will be a nightmare." - So why do it?

Comment 28

[Richard Marti (:Paenglab)]

(In reply to Alessandro Castellani (:aleca) from comment #25)

Sweet, thanks for starting on this.
Indeed, as Geoff pointed out, we shouldn't land this until we have the Search Box implemented and usable, otherwise scrolling through the preferences will be a nightmare.

I'm against it. This will take the AM in pref tab way: Not going further because it's not checked-in. We should land it, it's then in non-official builds and has time to grow until 75, the next official build.

For the search we should do this in three steps: 1. land this bug. 2. convert it to the FX way: a big xhtml <page> with no <prefpane>. 3. now implement the search.

With this way the search should easy to implement as we can more or less copy the FX search implementation.

I'll finish this patches here. But if you don't want to land them, I'm stopping then the maintenance to update this patches and someone other can un-bitrot this patches all the time until it lands. Again, we don't land it on ESR soon, it's then on nightly and later on beta.

Comment 29

[Richard Marti (:Paenglab)]

Jörg, see the Usability Study at https://ura.design/projects/thunderbird

Comment 30

[Alessandro Castellani [:aleca]]

Thanks Richard for the clarification.
I see your point and it makes sense, considering it won't land on ESR but it'll stay on nightly until 75.

Upon further inspection of the various sections, I noticed that we have many tabs with only one or two options, so listing them all in one page won't be a problem.

Or to us the words of TB's lead UX engineer: " ... scrolling through the preferences will be a nightmare." - So why do it?

I should have phrase that better, my bad.
I assumed the single panels would turn into an infinite list of options, but that won't be the case.
Many of our tabs are almost empty, and as emerged from the usability study, splitting content into tabs is not the best for accessibility and discoverability.

Comment 31

[Magnus Melin [:mkmelin]]

Agreed this looks great, and we don't need to have more than the basics for it to land and we do have time to improve it adding a search before 76.

Comment 32

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-1.patch

Okay, this part is ready for review.
Geoff, you made a lot on the prefs. So I think you're the right to review this.

The patch moved the parts to General pane. The other panes are still working, only without the moved parts.

In preparation to the move to the FX implementation I already added the data-category="paneGeneral" which will be used to show/hide the parts depending of the selected category.

Comment 33

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-2.patch

This patch should work except the Default startup directory menulist which is empty. When selecting a directory it is shown in the menulist, but after closing/opening of the prefs it is empty again. I see no errors in the console.

Aceman, please can you look what is wrong with my patch? You need to apply 1498332-rearrange-prefs-part-1.patch first.

Comment 34

[Alessandro Castellani [:aleca]]

Comment on attachment 9075228 [details] [diff] [review]
1498332-rearrange-prefs-part-1.patch

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

Great work, this is starting to look really neat.
Here's my UI review with mostly nit changes, just to tighten up the whole area and improve visual consistency.

**Fields spacing**
There's some inconsistency when a field is inline with other elements, like buttons or another field.
We should follow what Firefox does, having always the same spacing in between aligned elements.
It should be a `4px` gap if I'm not wrong. (red lines on the attachment)

**Sections spacing**
The `Language and Appearance`  section has really great spacing in between option groups. We should try to maintain that visual consistency by separating other areas with the same approach.
The Reading and Display section is the most cramped of them all. (purple lines on attachment)

**String titles**
Sometimes we use `and` and sometimes we use `&` for section or group titles.
We should decide which approach we want to use and keep it consistent.

I'm uploading a full screenshot with some notes on top for spacing, so you have a visual reference of what I'm talking about.

Let's shrink down a bit the overall font of the panel
```
.dialogBox, #MailPreferences prefpane {
    font-size: 1.3em;
}
```

::: mail/themes/shared/mail/incontentprefs/aboutPreferences.css
@@ +66,1 @@
>    font-size: 1.14em;

This feels a bit bloated, let's shrink it down to `1.1em`

Comment 35

[Alessandro Castellani [:aleca]]

Attachment: Preferences.png

Comment 36

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-1.patch

Ui-r comments should be addressed.

Comment 37

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-2.patch

Aceman, I updated this patch to apply after part 1. My issue is still the Default startup directory menulist which is empty. When selecting a directory it is shown in the menulist, but after closing/opening of the prefs it is empty again. I see no errors in the console.

Please help me to find the issue.

Comment 38

[Alessandro Castellani [:aleca]]

Comment on attachment 9075645 [details] [diff] [review]
1498332-rearrange-prefs-part-1.patch

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

Nice, it looks way better with those tweaks.
You missed the Reading & Display section. We need some vertical space before "Open messages in:" and "Display name:"

ui-r+ after that small tweak.

Comment 39

[Richard Marti (:Paenglab)]

Comment on attachment 9075651 [details] [diff] [review]
1498332-rearrange-prefs-part-2.patch

Alessandro, maybe you can already do the ui-r. There should be no visual change when fixing the menulist issue.

Comment 40

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-1.patch

Thank you, Alessandro. Fixed the two remaining spacings.

Comment 41

[Alessandro Castellani [:aleca]]

Comment on attachment 9075651 [details] [diff] [review]
1498332-rearrange-prefs-part-2.patch

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

Nice work!
Would be possible to change the icon used in the sidebar? "Composition" still uses the old pencil

Comment 42

[Richard Marti (:Paenglab)]

(In reply to Alessandro Castellani (:aleca) from comment #41)

Would be possible to change the icon used in the sidebar? "Composition"
still uses the old pencil

I'll do it in part 4 where I do the CSS cleanup and remove the no more used icons.

Comment 43

[Geoff Lankow (:darktrojan)]

Comment on attachment 9075738 [details] [diff] [review]
1498332-rearrange-prefs-part-1.patch

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

Fine, with some comments. On the whole, given that this is mostly cut-and-paste, I probably looked too closely at the XUL and not closely enough at the JS.

You'll need to adjust the mochitests before landing this.

::: mail/components/preferences/advanced.js
@@ +39,1 @@
>  

The linter doesn't like this empty line.

::: mail/components/preferences/compose.inc.xul
@@ +3,5 @@
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>    <prefpane id="paneCompose" label="&paneComposition.title;">
>      <script src="chrome://messenger/content/preferences/compose.js"/>
>      <script src="chrome://global/content/contentAreaUtils.js"/>
> +    <script src="chrome://messenger/content/preferences/downloads.js"/>

What's this doing here?

::: mail/components/preferences/general.inc.xul
@@ +7,5 @@
> +    <script src="chrome://mozapps/content/preferences/fontbuilder.js"/>
> +    <script>
> +    <![CDATA[
> +#ifdef MOZ_WIDGET_GTK
> +    var ICON_URL_APP = "moz-icon://dummy.exe?size=16";

Let's do this with AppConstants.MOZ_WIDGET_GTK and not use preprocessing and an inline script.

@@ +34,4 @@
>  
>      <stringbundle id="bundlePreferences" src="chrome://messenger/locale/preferences/preferences.properties"/>
> +#ifdef HAVE_SHELL_SERVICE
> +    <stringbundle id="bundlePreferences" src="chrome://messenger/locale/preferences/preferences.properties"/>

This stringbundle is already here.

We should probably stop using <stringbundle> altogether as we change this code (and use Services.strings instead), but whether you do that or not is up to you.

@@ +118,5 @@
> +        <vbox id="fontRow" flex="1">
> +          <hbox align="center">
> +            <label accesskey="&defaultFont.accesskey;" control="defaultFont">&defaultFont.label;</label>
> +            <menulist id="defaultFont" flex="1" sizetopopup="pref" crop="center"
> +                      onsyncfrompreference="return gGeneralPane.readFontSelection()">

Bug 1562560 is going to cause a conflict here. I haven't started it yet, but probably should.

@@ +161,5 @@
> +                  accesskey="&colorButton.accesskey;" oncommand="gGeneralPane.configureColors();"/>
> +        </vbox>
> +      </hbox>
> +      <hbox>
> +      <label><html:h2>&displayWidth.label;</html:h2></label>

Indentation.

@@ +208,5 @@
> +        <hbox>
> +          <radio id="appLocale"
> +                  value="false"/>
> +                  <!-- label and accesskey will be set dynamically -->
> +          <spacer flex="1"/>

I know this is cut-and-pasted, but I'm going to pick on it anyway. :-)

The <spacer>s are redundant. The outer <hbox> is redundant (as well as messing up the indent). If the <radiogroup> was align="start", the inner <hbox>es would be redundant.

In fact, I see this pattern all over the place. Does it do something I'm not seeing?

@@ +225,5 @@
> +      <label><html:h2>&languageSelector.label;</html:h2></label>
> +        <vbox align="start">
> +        <description flex="1"
> +                      controls="chooseMessengerLanguage"
> +                      data-l10n-id="choose-messenger-language-description"/>

Indentation.

@@ +392,5 @@
> +      <vbox align="start">
> +        <radiogroup id="saveWhere" flex="1"
> +                    preference="browser.download.useDownloadDir"
> +                    onsyncfrompreference="return gDownloadDirSection.onReadUseDownloadDir();">
> +        <hbox id="saveToRow" align="center">

Indentation of both these hboxes.

@@ +488,5 @@
> +      </vbox>
> +
> +      <vbox>
> +
> +      <separator/>

Shouldn't this be outside the vbox?

@@ +701,5 @@
> +      </hbox>
> +    </groupbox>
> +
> +    <groupbox data-category="paneGeneral">
> +        <label><html:h2>&Diskspace;</html:h2></label>

Indentation.

Comment 44

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-1.patch

(In reply to Geoff Lankow (:darktrojan) from comment #43)

Comment on attachment 9075738 [details] [diff] [review]
1498332-rearrange-prefs-part-1.patch

Review of attachment 9075738 [details] [diff] [review]:

Fine, with some comments. On the whole, given that this is mostly
cut-and-paste, I probably looked too closely at the XUL and not closely
enough at the JS.

You'll need to adjust the mochitests before landing this.

I'll plan to look when all patches are ready. I hope I can fix them myself.

::: mail/components/preferences/advanced.js
@@ +39,1 @@

The linter doesn't like this empty line.

Fixed. Before I thought I'll let it like it is because this file will be deleted. But better it's clean.

::: mail/components/preferences/compose.inc.xul
@@ +3,5 @@

file, You can obtain one at http://mozilla.org/MPL/2.0/.

<prefpane id="paneCompose" label="&paneComposition.title;">
<script src="chrome://messenger/content/preferences/compose.js"/>
<script src="chrome://global/content/contentAreaUtils.js"/>

• <script src="chrome://messenger/content/preferences/downloads.js"/>

What's this doing here?

I had to insert it after contentAreaUtils.js to not get the error from comment 22.
I'll plan to move all <script> to aboutPreferences.xul in a follow-up bug.

::: mail/components/preferences/general.inc.xul
@@ +7,5 @@

• <script src="chrome://mozapps/content/preferences/fontbuilder.js"/>
• <script>
• <![CDATA[
  +#ifdef MOZ_WIDGET_GTK
• var ICON_URL_APP = "moz-icon://dummy.exe?size=16";

Let's do this with AppConstants.MOZ_WIDGET_GTK and not use preprocessing and
an inline script.

Done.

@@ +34,4 @@

 <stringbundle id="bundlePreferences" src="chrome://messenger/locale/preferences/preferences.properties"/>

+#ifdef HAVE_SHELL_SERVICE

• <stringbundle id="bundlePreferences" src="chrome://messenger/locale/preferences/preferences.properties"/>

This stringbundle is already here.

I must be blind. :-(

We should probably stop using <stringbundle> altogether as we change this
code (and use Services.strings instead), but whether you do that or not is
up to you.

Better not in this bug to not mess too much.

@@ +208,5 @@

•    <hbox>
  

•      <radio id="appLocale"
  

•              value="false"/>
  

•              <!-- label and accesskey will be set dynamically -->
  

•      <spacer flex="1"/>
  

I know this is cut-and-pasted, but I'm going to pick on it anyway. :-)

The <spacer>s are redundant. The outer <hbox> is redundant (as well as
messing up the indent). If the <radiogroup> was align="start", the inner
<hbox>es would be redundant.

Both fixed.

In fact, I see this pattern all over the place. Does it do something I'm not
seeing?

It was to make not the whole line selectable. I can fix them in a follow-up bug.

Indentation.

Fixed all indentations you found.

@@ +488,5 @@

•  </vbox>
  

•  <vbox>
  

•  <separator/>
  

Shouldn't this be outside the vbox?

Moved.

I found an issue with the applicationManager and fixed it (still pointed to gApplicationsPane instead to gGeneralPane).

Comment 45

[:aceman]

Ah, another change I see no benefit in :)

(In reply to Richard Marti (:Paenglab) from comment #37)

Aceman, I updated this patch to apply after part 1. My issue is still the Default startup directory menulist which is empty. When selecting a directory it is shown in the menulist, but after closing/opening of the prefs it is empty again. I see no errors in the console.

The menulist id=defaultStartupDirList does not contain any items when initAbDefaultStartupDir() is run thus no item is selected and the menulist appears empty. Only later after that the menulist is populated with the list of addressbook as the connectedCallback() of that particular menulist is run.

It seems the "paneload" event (which runs the init functions of the panes) is run sooner than the custom element is built. I don't know when exactly custom elements load and build their contents, but it's not the first problem of their weird timing.

Comment 46

[Richard Marti (:Paenglab)]

Thanks Aceman. Geoff, do you know how this could be solved?

Comment 47

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-3.patch

I think also with the menulist problem in part 2 we can go further with the ui-r.

This patch implements the "Privacy & Security" pane.

Comment 48

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-4.patch

This is the patch for the "Chat" pane. Now with the fixed chat crash this works too without additional back-out patch.

Comment 49

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-5.patch

This is the "Calendar" part.
The mockup forgot the Categories. I added them at the bottom of the pane, okay?

Comment 50

[:aceman]

Also notice the long page can't be scrolled by Home/End/Page Up/Page Down, unless you click some option. It is not enough to click any empty space inside the page, as would work in any other web page.
But this bug is in Firefox too.

Comment 51

[Geoff Lankow (:darktrojan)]

The menulist id=defaultStartupDirList does not contain any items when initAbDefaultStartupDir() is run thus no item is selected and the menulist appears empty. Only later after that the menulist is populated with the list of addressbook as the connectedCallback() of that particular menulist is run.

It looks like putting setTimeout(…) around the call to that function from gComposePane.init does the trick.

Comment 52

[:aceman]

Maybe we could run whole gComposePane.init from a timeout.
Anyway those are all hack on hacks. What is the proper way to handle custom elements and run after they are already initialized?
We have more problems like this in the preferences pane (e.g. opening the Prefs tab always throws a problem with this.mSelectedInternal from some uninitialized menulist).

Comment 53

[Richard Marti (:Paenglab)]

I tried the setTimeout(…) this weekend it didn't work for me. How should the command look and which value did you use?

Comment 54

[Alessandro Castellani [:aleca]]

Attachment: preferences-spacing.png

Comment 55

[Alessandro Castellani [:aleca]]

Comment on attachment 9076374 [details] [diff] [review]
1498332-rearrange-prefs-part-3.patch

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

For the **Privacy** section, increase the spacing between `Mail Content` and `Web Content`. (screenshot above)
Same spacing we're using in between sections in the General tab.

::EDIT::

Sorry, the patch didn't properly apply, disregard the screenshot.
This looks good!

ui-r+

Comment 56

[Alessandro Castellani [:aleca]]

Attachment: chat-spacing.png

Comment 57

[Alessandro Castellani [:aleca]]

Comment on attachment 9076375 [details] [diff] [review]
1498332-rearrange-prefs-part-4.patch

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

Increase the spacing before the "When message directed at you arrive:" text. (screenshot above)

Other than that it looks good.
ui-r+ after this fix.

Comment 58

[Alessandro Castellani [:aleca]]

Comment on attachment 9076376 [details] [diff] [review]
1498332-rearrange-prefs-part-5.patch

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

::: calendar/base/content/preferences/alarms.inc.xul
@@ +186,4 @@
>                              </menupopup>
>                          </menulist>
> +                    </hbox>
> +                </html:td>

I'm not sure about this section.
We never aligned fields of multiple similar rows to the right.
So far we have a visual consistency with fields and dropdowns aligned inline to their labels, with sometime the field growing at 100%.
The right side has been always reserved for buttons.

::: calendar/base/content/preferences/views.inc.xul
@@ +49,5 @@
>              </hbox>
> +        </hbox>
> +        <hbox align="center" flex="1">
> +            <checkbox id="weekNumber"
> +                      class="indent"

I don't think we should indent this checkbox as it's not a sub-preference or grouped underneath the title above itself.

@@ +54,5 @@
> +                      crop="end"
> +                      label="&pref.calendar.view-minimonth.showweeknumber.label;"
> +                      accesskey="&pref.calendar.view-minimonth.showweeknumber.accesskey;"
> +                      preference="calendar.view-minimonth.showWeekNumber"/>
> +        </hbox>

Increase the spacing between these two containers. There a lot of checkboxes here and we need a bit more breathing room.

@@ +150,5 @@
> +                        </menupopup>
> +                    </menulist>
> +                </html:td>
> +                <html:td>
> +                    <hbox align="center" pack="center">

This element feels a bit weird all alone in the middle.
We never used these type of alignment or columns, so I think we should put this option on its own line, underneath the Day ends option.

Comment 59

[Geoff Lankow (:darktrojan)]

(In reply to :aceman from comment #52)

Maybe we could run whole gComposePane.init from a timeout.
Anyway those are all hack on hacks. What is the proper way to handle custom elements and run after they are already initialized?
We have more problems like this in the preferences pane (e.g. opening the Prefs tab always throws a problem with this.mSelectedInternal from some uninitialized menulist).

I don't know about proper ways to do this, but I think it's happening because all the initialisation code is running as soon as the parser gets to it. Does that prevent the CE bindings from applying beforehand? I guess so.

We've already got a function at the top of preferences.js which if put in a no-delay timeout should run after the CE stuff.

Comment 60

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-2.patch

I added now the setTimeout() hack with 1 ms delay which works for me, is more needed for slower systems? I hope, this is okay for now and a better solution can be found in a new bug.

Patch is also updated after landing of bug 1564294.

Comment 61

[Richard Marti (:Paenglab)]

Comment on attachment 9076374 [details] [diff] [review]
1498332-rearrange-prefs-part-3.patch

I saw no issues here and this patch should be okay for review.

Comment 62

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-4.patch

Added a <separator/> before the "When message directed at you arrive:" text.

Comment 63

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-5.patch

Alessandro, does this look better?

Comment 64

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-1.patch

Updated after bug 1562560.

Comment 65

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-2.patch

Comment 66

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-3.patch

Comment 67

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-4.patch

Comment 68

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-5.patch

Comment 69

[Geoff Lankow (:darktrojan)]

(In reply to Richard Marti (:Paenglab) from comment #60)

I added now the setTimeout() hack with 1 ms delay which works for me, is more needed for slower systems? I hope, this is okay for now and a better solution can be found in a new bug.

It's not a timing issue, it's an order of operations issue. setTimeout even with 0 delay is basically saying "do this when you've finished doing everything else".

I think what we should do is put this line inside a timeout. That would delay all of the things listening for a paneload event, and also seems to solve another problem I've been noticing (that sometimes nothing appears on the tab if it's open when Thunderbird starts).

Comment 70

[Geoff Lankow (:darktrojan)]

Comment on attachment 9077455 [details] [diff] [review]
1498332-rearrange-prefs-part-2.patch

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

::: mail/components/preferences/applications.inc.xul
@@ -1,4 @@
> -# This Source Code Form is subject to the terms of the Mozilla Public
> -# License, v. 2.0. If a copy of the MPL was not distributed with this
> -# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> -  <prefpane id="paneApplications" label="&paneAttachments.title;">

You'll need to change a reference to this in downloads.js, that was added with bug 1562560.

::: mail/components/preferences/compose.inc.xul
@@ +70,3 @@
>  
> +      <hbox align="center" pack="start">
> +        <label value ="&languagePopup.label;" control="languageMenuList" accesskey="&languagePopup.accessKey;"/>

There's a stray space here. It's not your fault, but please fix it.

@@ +276,5 @@
> +                   href="https://addons.thunderbird.net/thunderbird/tag/filelink"
> +                   value="&findCloudFileProviders.label;"/>
> +          </vbox>
> +          <separator class="thin" orient="vertical"/>
> +          <vbox flex="1" style="height: 400px;">

This height needs to be on the parent box. If you add too many providers, the whole thing expands.

Additionally, the random image from the WeTransfer provider now overflows and you can't see the explanation text below it (not that it helps, often).

::: mail/components/preferences/compose.js
@@ +74,3 @@
>      }
>  
> +    setTimeout(function() {gComposePane.initAbDefaultStartupDir()}, 1);

See comment 69.

Comment 71

[Geoff Lankow (:darktrojan)]

I've noticed that when scrolled down in a pane, if you change pane the new pane is also scrolled down. That's not right.

Also the chat pane jumps to the bottom the first time it is shown.

Comment 72

[:aceman]

(In reply to Geoff Lankow (:darktrojan) from comment #69)

I think what we should do is put this line inside a timeout. That would delay all of the things listening for a paneload event, and also seems to solve another problem I've been noticing (that sometimes nothing appears on the tab if it's open when Thunderbird starts).

That's what I was aiming at in comment 52.

Comment 73

[Geoff Lankow (:darktrojan)]

Comment on attachment 9077456 [details] [diff] [review]
1498332-rearrange-prefs-part-3.patch

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

Nothing too exciting here.

::: mail/components/preferences/privacy.js
@@ +297,5 @@
> +   */
> +  openTextLink(evt) {
> +    // Opening links behind a modal dialog is poor form. Work around flawed
> +    // text-link handling by opening in browser if we'd instead get a content
> +    // tab behind the modal options dialog.

This can probably go. It's not a modal dialog any more!

@@ +328,5 @@
> +  initSubmitCrashes() {
> +    var checkbox = document.getElementById("submitCrashesBox");
> +    try {
> +      var cr = Cc["@mozilla.org/toolkit/crash-reporter;1"].
> +               getService(Ci.nsICrashReporter);

Let's line the dot up with the bracket while we're here.

@@ +346,5 @@
> +  updateSubmitCrashes() {
> +    var checkbox = document.getElementById("submitCrashesBox");
> +    try {
> +      var cr = Cc["@mozilla.org/toolkit/crash-reporter;1"].
> +               getService(Ci.nsICrashReporter);

And this one.

::: mail/themes/shared/mail/incontentprefs/aboutPreferences.css
@@ -243,5 @@
>  radio[pane=paneCompose] > .radio-check {
>    list-style-image: url("chrome://messenger/skin/shared/in-content/compose.svg");
>  }
>  
>  radio[pane=paneAdvanced] > .radio-check {

Unused now?

Comment 74

[Geoff Lankow (:darktrojan)]

Comment on attachment 9077457 [details] [diff] [review]
1498332-rearrange-prefs-part-4.patch

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

::: mail/components/preferences/chat.inc.xul
@@ +17,5 @@
> +    <groupbox data-category="paneChat">
> +      <label><html:h2 data-l10n-id="chat-status-title"/></label>
> +        <!-- Startup -->
> +        <hbox align="center">
> +          <label id ="chatStartupAction" value="&startupAction.label;"

Another stray space. I don't know why these stand out so much.

::: mail/locales/en-US/messenger/preferences/preferences.ftl
@@ +50,5 @@
>  privacy-certificates-title = Certificates
>  
> +chat-pane-header = Chat
> +
> +chat-status-title =Status

Space.

::: mail/themes/shared/mail/incontentprefs/aboutPreferences.css
@@ -240,5 @@
> -  list-style-image: url("chrome://messenger/skin/shared/in-content/compose.svg");
> -}
> -
> -radio[pane=paneAdvanced] > .radio-check {
> -  list-style-image: url("chrome://messenger/skin/shared/in-content/advanced.svg");

Oh, you have done it. Never mind! :-)

Comment 75

[Geoff Lankow (:darktrojan)]

Comment on attachment 9077458 [details] [diff] [review]
1498332-rearrange-prefs-part-5.patch

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

This pane looks a lot better than it did. Well done.

::: calendar/lightning/content/messenger-overlay-preferences.xul
@@ +33,5 @@
>                flex="1"
>                insertbefore="paneAdvanced"
>                label="&lightning.preferencesLabel;">
> +
> +    <hbox id="chatPaneCategory"

These are all id="chatPaneCategory". And now that I've noticed it, there's two in chat.inc.xul with the same id.

::: calendar/locales/en-US/chrome/calendar/preferences/general.dtd
@@ +19,5 @@
>  <!ENTITY pref.defaults.label "Default Values for New Items">
>  <!ENTITY pref.events.label "Events">
>  <!ENTITY pref.tasks.label "Tasks">
>  
> +<!ENTITY pref.default_event_task_length.label "Default Event and Tasks Length">

Task should be singular.

::: calendar/locales/en-US/chrome/calendar/preferences/views.dtd
@@ +6,5 @@
>     - If this ==> … <== doesn't look like an ellipsis (three dots in a row),
>     - your editor isn't using UTF-8 encoding and may munge up the document!
>    -->
>  
> +<!ENTITY pref.calendar.view.format.caption "Formats">

I don't think Formats is really the right label, and I wonder if we even need one in this spot.

Comment 76

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-1.patch

Added the
setTimeout(function() {pane.dispatchEvent(new CustomEvent("paneload"))}, 1);
and the
document.getElementById("preferencesContainer").scrollTo(0, 0);
to preferences.js.

Comment 77

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-2.patch

(In reply to Geoff Lankow (:darktrojan) from comment #70)

Comment on attachment 9077455 [details] [diff] [review]
1498332-rearrange-prefs-part-2.patch

Review of attachment 9077455 [details] [diff] [review]:

::: mail/components/preferences/applications.inc.xul
@@ -1,4 @@

-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.

• <prefpane id="paneApplications" label="&paneAttachments.title;">

You'll need to change a reference to this in downloads.js, that was added
with bug 1562560.
fixed
::: mail/components/preferences/compose.inc.xul
@@ +70,3 @@

•  <hbox align="center" pack="start">
  

•    <label value ="&languagePopup.label;" control="languageMenuList" accesskey="&languagePopup.accessKey;"/>
  

There's a stray space here. It's not your fault, but please fix it.
fixed
@@ +276,5 @@

•               href="https://addons.thunderbird.net/thunderbird/tag/filelink"
  

•               value="&findCloudFileProviders.label;"/>
  

•      </vbox>
  

•      <separator class="thin" orient="vertical"/>
  

•      <vbox flex="1" style="height: 400px;">
  

This height needs to be on the parent box. If you add too many providers,
the whole thing expands.
Moved to the parent <hbox>
Additionally, the random image from the WeTransfer provider now overflows
and you can't see the explanation text below it (not that it helps, often).
Made it 480px height.
::: mail/components/preferences/compose.js
@@ +74,3 @@

 }

• setTimeout(function() {gComposePane.initAbDefaultStartupDir()}, 1);

See comment 69.
Is now in patch-1.

Comment 78

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-3.patch

(In reply to Geoff Lankow (:darktrojan) from comment #73)

Comment on attachment 9077456 [details] [diff] [review]
1498332-rearrange-prefs-part-3.patch

Review of attachment 9077456 [details] [diff] [review]:

Nothing too exciting here.

::: mail/components/preferences/privacy.js
@@ +297,5 @@

• */
• openTextLink(evt) {
• // Opening links behind a modal dialog is poor form. Work around flawed
• // text-link handling by opening in browser if we'd instead get a content
• // tab behind the modal options dialog.

This can probably go. It's not a modal dialog any more!
Removed the function as the is="text-link" does the opening of the link.
@@ +328,5 @@

• initSubmitCrashes() {
• var checkbox = document.getElementById("submitCrashesBox");
• try {

•  var cr = Cc["@mozilla.org/toolkit/crash-reporter;1"].
  

•           getService(Ci.nsICrashReporter);
  

Let's line the dot up with the bracket while we're here.
Done
@@ +346,5 @@

• updateSubmitCrashes() {
• var checkbox = document.getElementById("submitCrashesBox");
• try {

•  var cr = Cc["@mozilla.org/toolkit/crash-reporter;1"].
  

•           getService(Ci.nsICrashReporter);
  

And this one.
Done

Comment 79

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-4.patch

(In reply to Geoff Lankow (:darktrojan) from comment #74)

Comment on attachment 9077457 [details] [diff] [review]
1498332-rearrange-prefs-part-4.patch

Review of attachment 9077457 [details] [diff] [review]:

::: mail/components/preferences/chat.inc.xul
@@ +17,5 @@

• <groupbox data-category="paneChat">

•  <label><html:h2 data-l10n-id="chat-status-title"/></label>
  

•    <!-- Startup -->
  

•    <hbox align="center">
  

•      <label id ="chatStartupAction" value="&startupAction.label;"
  

Another stray space. I don't know why these stand out so much.
Fixed
::: mail/locales/en-US/messenger/preferences/preferences.ftl
@@ +50,5 @@

privacy-certificates-title = Certificates

+chat-pane-header = Chat
+
+chat-status-title =Status

Space.
Fixed

Comment 80

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-5.patch

(In reply to Geoff Lankow (:darktrojan) from comment #75)

Comment on attachment 9077458 [details] [diff] [review]
1498332-rearrange-prefs-part-5.patch

Review of attachment 9077458 [details] [diff] [review]:

This pane looks a lot better than it did. Well done.

::: calendar/lightning/content/messenger-overlay-preferences.xul
@@ +33,5 @@

           flex="1"
           insertbefore="paneAdvanced"
           label="&lightning.preferencesLabel;">

• <hbox id="chatPaneCategory"

These are all id="chatPaneCategory". And now that I've noticed it, there's
two in chat.inc.xul with the same id.
Fixed here and in Chat.
::: calendar/locales/en-US/chrome/calendar/preferences/general.dtd
@@ +19,5 @@

<!ENTITY pref.defaults.label "Default Values for New Items">
<!ENTITY pref.events.label "Events">
<!ENTITY pref.tasks.label "Tasks">

+<!ENTITY pref.default_event_task_length.label "Default Event and Tasks Length">

Task should be singular.
Fixed
::: calendar/locales/en-US/chrome/calendar/preferences/views.dtd
@@ +6,5 @@

- If this ==> … <== doesn't look like an ellipsis (three dots in a row),
- your editor isn't using UTF-8 encoding and may munge up the document!

-->

+<!ENTITY pref.calendar.view.format.caption "Formats">

I don't think Formats is really the right label, and I wonder if we even
need one in this spot.
I removed it.

Comment 81

[Richard Marti (:Paenglab)]

I'm now looking for the tests.

Comment 82

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-1.patch

Shoot, introduced 3 linting errors in one line with the setTimeout(function() { pane.dispatchEvent(new CustomEvent("paneload")); }, 1);

Comment 83

[Richard Marti (:Paenglab)]

Attachment: 1498332-fix-tests.patch

I tried what I could with my limited knowledge.
See https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d6cbe5f4eb206263c5d1c1bd8d421556d22f9f20

Aceman, Geoff, please could you look what is wrong with my tests?

I've set for the no more existing tabs in the pane the undefined like https://searchfox.org/comm-central/source/mail/components/preferences/test/browser/browser_privacy.js#6 already had. I think this works now but something else is wrong in the tests.

For removing the aTabID I filed bug 1565789.

Comment 84

[Geoff Lankow (:darktrojan)]

Once bug 1565789 is done, you should use it in the tests, as it looks like a lot of the failures are from clicking on things that are off-screen.

Comment 85

[Richard Marti (:Paenglab)]

Attachment: 1498332-fix-tests.patch

This is my latest test patch with the scrollTo elements for bug 1565789.
What do I wrong?
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=fc3dec7218b86e76dd15cdfb0ece357e7aad64b5

Or please, could you look to fix it?

Comment 86

[Jorg K (CEST = GMT+2)]

Looks like the only test failing is testAlarmDefaultValue.js. And the linter tells us what the problem is:
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/test/mozmill/testAlarmDefaultValue.js:35:8 | 'content_tab_e' is not defined. (no-undef)

Comment 87

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #86)

Looks like the only test failing is testAlarmDefaultValue.js. And the linter tells us what the problem is:
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/test/mozmill/testAlarmDefaultValue.js:35:8 | 'content_tab_e' is not defined. (no-undef)

Changed

• ({ content_tab_e, content_tab_eid } = collector.getModule("content-tab-helpers"));

• content_tab_eid = collector.getModule("content-tab-helpers");

But what about the bct errors and the Z2 and Z8 errors in debug?

Comment 88

[Jorg K (CEST = GMT+2)]

Z8 is the same issue, Z2 seems intermittent, and bct is an issue, indeed.

Comment 89

[Jorg K (CEST = GMT+2)]

You need to rerun this, it was totally busted due to bug 1566338.

Comment 90

[Geoff Lankow (:darktrojan)]

I'll take a look at these tests tomorrow. There's a number of things to fix up.

Comment 91

[Richard Marti (:Paenglab)]

Attachment: 1498332-fix-tests.patch

ESLint is now satisfied but the calendar error still exists.

Comment 92

[Richard Marti (:Paenglab)]

Attachment: 1498332-rearrange-prefs-part-4.patch

Fixed the not centred "Fonts for:" title and removed a no more needed rule.

Comment 93

[Geoff Lankow (:darktrojan)]

Attachment: 1498332-fix-tests.patch

I've fixed the remaining test failures. There's some refactoring that could be done, but as it's test code and it works, I don't really care about that.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=df0cf1bcabc975003d0a29708cf7ef7e2c85594b

Comment 94

[Richard Marti (:Paenglab)]

Many thanks for fixing the tests, Geoff.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=df0cf1bcabc975003d0a29708cf7ef7e2c85594b looks good -> c-n

This needs to land together with bug 1565789.

Comment 95

[Jorg K (CEST = GMT+2)]

Sorry, I'm confused here. Does bug 1565789 need go first or after part 5 as Geoff had it in his try?

Bug 1498332 - Fix the tests after the prefs rearrange. r=darktrojan
Bug 1565789 - Update the preferences callers after the prefs redesign in Bug 1498332. r=darktrojan
Bug 1498332 - Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca
Bug 1498332 - Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca
Bug 1498332 - Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca
Bug 1498332 - Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca
Bug 1498332 - Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca

What about the stufftofix patch, was this merged into the test-fix patch here? I can't see, for example, these changes:
https://hg.mozilla.org/try-comm-central/rev/09bd26f4103b4a598a86af950dc39c84de30722b#l1.12
in the patch.

Comment 96

[Jorg K (CEST = GMT+2)]

Does bug 1565789 need go first or after part 5 as Geoff had it in his try?

If it needs to be shuffled like this, or can it go first, but then the commit message isn't right:
"Update the preferences callers after the prefs redesign in bug 1498332."

The test patch doesn't apply if the patch from bug 1565789 does't come first in some way.

Comment 97

[Richard Marti (:Paenglab)]

The order of the two bugs doesn't matter. They have to go together to not get test errors.

Comment 98

[Jorg K (CEST = GMT+2)]

Not quite. 1565789 need to be applied or else Geoff's test changes patch doesn't apply. But I'm happy to put it first and change the commit message. Anyway, I'm still unsure what to do with this:
https://hg.mozilla.org/try-comm-central/rev/09bd26f4103b4a598a86af950dc39c84de30722b
since this was part of the try run but doesn't appear to be part of the patch.

Comment 99

[Geoff Lankow (:darktrojan)]

I folded my changes into the last patch and uploaded the combined patch.

Comment 100

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3440c68aebee
Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca
https://hg.mozilla.org/comm-central/rev/4aac8099a1ed
Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca
https://hg.mozilla.org/comm-central/rev/719b71ad6690
Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca
https://hg.mozilla.org/comm-central/rev/0795c763e706
Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca
https://hg.mozilla.org/comm-central/rev/5f73dd30816c
Redesign and rearrange Thunderbird Preferences/Options similarly to Firefox. r=darktrojan, ui-r=aleca
https://hg.mozilla.org/comm-central/rev/62bf2165d66e
Fix the tests after the prefs rearrange. r=darktrojan

Comment 101

[Jorg K (CEST = GMT+2)]

So now we integrate the Account Manager into this?

Comment 102

[Alessandro Castellani [:aleca]]

(In reply to Jorg K (GMT+2) from comment #101)

So now we integrate the Account Manager into this?

We should first implement the Search Box, and then move to the Account Manager.

I'm also getting this error since this patch landed whenever I close the preferences:
JavaScript error: resource://gre/modules/SessionStoreFunctions.jsm, line 122: NS_ERROR_FILE_NOT_FOUND:

Is it related or known?

Comment 103

[Richard Marti (:Paenglab)]

(In reply to Alessandro Castellani (:aleca) from comment #102)

I'm also getting this error since this patch landed whenever I close the preferences:
JavaScript error: resource://gre/modules/SessionStoreFunctions.jsm, line 122: NS_ERROR_FILE_NOT_FOUND:
Is it related or known?

This file is only referenced in m-c. This is more related to bug 1563171 which landed two days ago.

Comment 104

[Jorg K (CEST = GMT+2)]

Removed the AM from the summary since it was misleading.

Comment 105

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/83dc7149848f
Actually remove test file that was made obsolete. rs=me