1679255 - remove <deck> from migration.xhtml

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Magnus Melin [:mkmelin]]

Remove <deck> from https://searchfox.org/comm-central/rev/f8c11a36225bbe5e145f1a9a0a3801428e265715/mail/components/migration/content/migration.xhtml#30-47

It only has two children, https://searchfox.org/comm-central/rev/f8c11a36225bbe5e145f1a9a0a3801428e265715/mail/components/migration/content/migration.xhtml#31,46

These we can show/hide as needed. May need a min-height is one isn't set.

Comment 1

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679255_de-deck-migration-xhtml-0.patch

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9189713 [details] [diff] [review]
Bug-1679255_de-deck-migration-xhtml-0.patch

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

::: mail/components/migration/content/migration.js
@@ +156,4 @@
>  
>      if (firstNonDisabled) {
>        this._wiz.canAdvance = true;
> +      document.getElementById("importSourceNotFound").hidden = true;

Doesn't it need to unhide the other one?

Comment 3

[Khushil Mistry [:khushil324]]

Both are shown by default and we will hide one of them according to the window arguments.

Comment 4

[Alessandro Castellani [:aleca]]

Comment on attachment 9189713 [details] [diff] [review]
Bug-1679255_de-deck-migration-xhtml-0.patch

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

I think would be better to approach this from the other side, meaning we should start with both items hidden and only reveal what we need.

Since all the labels use textContent and not value, starting with all the elements hidden won't affect sizing or text wrapping.
The dialog is also sized by itself, so having both items visible by default is not very useful, and we might stumble upon a "flashing" UI if for whatever reason the JS code is delayed.

::: mail/components/migration/content/migration.xhtml
@@ +39,2 @@
>  #ifdef XP_WIN
> +          <radio id="outlook"   label="&importFromOutlook.label;"   accesskey="&importFromOutlook.accesskey;"/>

nit: remove the extra empty characters before the label and accesskey attributes and indent them to respect the max character limit.

Comment 5

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679255_de-deck-migration-xhtml-1.patch

Comment 6

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679255_de-deck-migration-xhtml-2.patch

Comment 7

[Alessandro Castellani [:aleca]]

Comment on attachment 9189938 [details] [diff] [review]
Bug-1679255_de-deck-migration-xhtml-2.patch

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

Good stuff, thanks for taking care of this.
Please, update the commit message with "Bug 1679255 - Remove the <deck> XUL element from the migration.xhtml dialog. r=aleca"

::: mail/components/migration/content/migration.js
@@ +160,1 @@
>      } else {

nit: here you can use a return; and avoid the else callback since no other code needs to run after the condition.

Comment 8

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679255_de-deck-migration-xhtml-3.patch

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/857e14a1cfe2
Remove the <deck> XUL element from the migration.xhtml dialog. r=aleca