1582718 - remove grid usage from comm/mail/components/compose/content/dialogs/EdSpellCheck.xul

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, task)

Comment 1

[Khushil Mistry [:khushil324]]

Attachment: Bug-1582718_remove-grid-EdSpellCheck.patch

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9097382 [details] [diff] [review]
Bug-1582718_remove-grid-EdSpellCheck.patch

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

I don't think you should use a table here. It's not tabular data. Maybe a grid layout?

::: mail/components/compose/content/dialogs/EdSpellCheck.xul
@@ +66,5 @@
> +  <hbox flex="1" id="edSpellCheckTableContainer">
> +    <html:table id="edSpellCheckTable">
> +      <html:tr id="suggestedListTr">
> +        <html:td id="suggestedListTd">
> +          <!-- BUG! setting class="MinWidth20em" on tree doesn't work (width=0) -->

don't know what this refers to, I think we can remove

Comment 3

[Khushil Mistry [:khushil324]]

Attachment: Bug-1582718-remove-grid-EdSpellCheck.patch

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 9098476 [details] [diff] [review]
Bug-1582718-remove-grid-EdSpellCheck.patch

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

There's some problem with things in the suggestions box. See screenshot. 

Also, the Check word button is misaligned, and not wide enough.

Also on trunk, but you could fix while here. If we have a very long word, it will enlarge the window without limit. Please set a max width for the misspelled word and text-overflow: ellipsis;

::: mail/themes/shared/mail/grid-layout.css
@@ +12,4 @@
>    grid-template-columns: auto auto auto;
>  }
>  
> +.grid-three-column-fr {

needs some better name.
.grid-three-column-auto-x-auto ?

Comment 5

[Magnus Melin [:mkmelin]]

Attachment: spellcheck-degrid.png

Comment 6

[Khushil Mistry [:khushil324]]

Attachment: Bug-1582718-remove-grid-EdSpellCheck.patch

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9098737 [details] [diff] [review]
Bug-1582718-remove-grid-EdSpellCheck.patch

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

The check word button is still of the wrong size. The other issues seems solved now.

::: mail/themes/linux/editor/EditorDialog.css
@@ +184,5 @@
>  }
>  
> +#MisspelledWord {
> +  max-width: 16em;
> +  text-overflow: ellipsis;

please put this inside the existing #MisspelledWord rule

Comment 8

[Khushil Mistry [:khushil324]]

Attachment: Screenshot from 2019-10-05 00-54-12.png

I am not able to see the size issue of the check button on Ubuntu 18.04.

Comment 9

[Magnus Melin [:mkmelin]]

Attachment: spell-ubuntu1904.png

This is on Ubuntu 19.04, not that it should matter.

Comment 10

[Khushil Mistry [:khushil324]]

Can you check with the CSS that what is wrong? I am not able to reproduce it on Mac and Linux both.

Comment 11

[Magnus Melin [:mkmelin]]

Comment on attachment 9098737 [details] [diff] [review]
Bug-1582718-remove-grid-EdSpellCheck.patch

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

::: mail/themes/linux/editor/EditorDialog.css
@@ +191,2 @@
>  .spell-check {
>    min-width: 8em;

if I make this 8.4em or larger, I don't see the problem. Maybe change it to 8.5em

Comment 12

[Khushil Mistry [:khushil324]]

Attachment: Bug-1582718-remove-grid-EdSpellCheck.patch

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/078fc8374bae
remove grid usage from EdSpellCheck.xul. r=mkmelin