1564270 - After inserting background image, dialogue needs to be OK'ed twice

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Jorg K (CEST = GMT+2)]

STR:
Start a new message.
Format > Page Color and Background.
Select a image from disk.
Hit OK. Image fills the background.
Hit OK again, panel dismissed.

Looks like some fallout from the accept dialogue changes.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: 1564270-page-background-1.diff

What a strange piece of code.

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 9076688 [details] [diff] [review]
1564270-page-background-1.diff

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

Thanks for looking into this quickly, just a  question.

::: editor/ui/dialogs/content/EdColorProps.js
@@ +401,5 @@
>        });
>        File.createFromNsIFile(nsFile).then(file => {
>          reader.readAsDataURL(file);
>        });
> +      if (event) {

Is this test necessary? Is it ever called with null/undefined?

@@ +419,2 @@
>    }
> +  if (event) {

And here?

Comment 3

[Jorg K (CEST = GMT+2)]

Comment on attachment 9076688 [details] [diff] [review]
1564270-page-background-1.diff

beta->69
release->68 (for the beta continuation I'll do for a week or so)

Comment 4

[Jorg K (CEST = GMT+2)]

Comment on attachment 9076688 [details] [diff] [review]
1564270-page-background-1.diff

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

::: editor/ui/dialogs/content/EdColorProps.js
@@ +397,2 @@
>          gDialog.BackgroundImageInput.value = reader.result;
>          if (onAccept()) {

Oops, is this recursively calling itself? And not passing event. Should be pass event and lose all the guards you're adding?

Comment 5

[Geoff Lankow (:darktrojan)]

Attachment: 1564270-page-background-2.diff

We can go that way if you like, it matters not…

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 9076961 [details] [diff] [review]
1564270-page-background-2.diff

If you don't mind, I prefer this.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5e8c4b40ca6a
Fix UI for setting a message's background image. r=jorgk DONTBUILD

Comment 8

[Jorg K (CEST = GMT+2)]

TB 69 beta (TB 68 beta coming):
https://hg.mozilla.org/releases/comm-beta/rev/75dffe1c92667a0b883fd6cc4b42aecc891bbb5d

Comment 9

[Jorg K (CEST = GMT+2)]

TB 68 beta 5:
https://hg.mozilla.org/releases/comm-beta/rev/1595c78e35881b6516a228e355f20405c7432ae6