1777788 - Send progress dialogue (sendProgress.xhtml) is collapsed to the title bar when a message is sent encrypted

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Rachel Martin]

First mentioned in bug 1774967 comment #2:

The send progress dialogue (sendProgress.xhtml) is collapsed to the title bar when a message is sent encrypted. This only seems to happen for small messages. With images attached, the send dialogue is shown in its full size. After that, the dialogue has the right size even for small messages, it returns to the erroneous "title bar size" after the next start of TB.

Seen in TB 102.0 on Windows.

Comment 1

[Henry Wilkes (they/them) [:henry-x]]

I can't reproduce on Daily on linux, after a restart, using openPGP and sending a plain text message to myself.

Given this isn't always produced on sending, this might be a regression triggered by bug 1703164, but as a side effect of bug 1752288

Are there any errors from the xul:dialog code in the console when this happens?

Comment 2

[Rachel Martin]

Try it on Windows. As for console errors, yes!

Uncaught TypeError: this._l10nButtons is undefined
    postLoadInit chrome://global/content/elements/dialog.js:324
    MozDialog chrome://global/content/elements/dialog.js:61
    sync chrome://openpgp/content/modules/cryptoAPI/interface.js:56
    encryptMessageStart chrome://openpgp/content/modules/encryption.jsm:440
    finishCryptoEncapsulation chrome://openpgp/content/modules/mimeEncrypt.jsm:559
    createMessageFile resource:///modules/MimeMessage.jsm:86
dialog.js:324:11
    postLoadInit chrome://global/content/elements/dialog.js:324
    MozDialog chrome://global/content/elements/dialog.js:61
    sync chrome://openpgp/content/modules/cryptoAPI/interface.js:56
    encryptMessageStart chrome://openpgp/content/modules/encryption.jsm:440
    finishCryptoEncapsulation chrome://openpgp/content/modules/mimeEncrypt.jsm:559
    createMessageFile resource:///modules/MimeMessage.jsm:86
    InterpretGeneratorResume self-hosted:1422
    AsyncFunctionNext self-hosted:632

Comment 3

[Henry Wilkes (they/them) [:henry-x]]

Yup. That looks like bug 1752288. I'll mark the regression as bug 1703164 though since that is what caused the bug to appear.

Comment 4

[b8]

Attachment: 1777788-send-window.png

In TB 109.0b2 this is not fixed despite bug 1752288 being fixed.

The send dialog is not properly sized on the first encrypted message sent after a new start of TB. It happens with an attachment of ~2MB. Previously only the title bar of the window was shown, now half the window is show with the "Cancel" button being cut off.

Note that Ctrl+Shift+Enter ("Send Later") was used for testing resulting in the "Saving Message" title.

Comment 5

[b8]

https://github.com/Betterbird/thunderbird-patches/blob/main/102/bugs/1777788-fix-dialog-resizing-m-c.patch
https://github.com/Betterbird/thunderbird-patches/blob/main/102/bugs/1777788-fix-dialog-size.patch
Maybe the first patch isn't necessary on trunk any more since a lot has changed in this area.

Comment 6

[Magnus Melin [:mkmelin]]

Can't reproduce. Maybe bug 1790619 will improve matters.

Comment 7

[Magnus Melin [:mkmelin]]

Or potentially style="width:100vw; height:100vh;" should be added to the dialog.

Comment 8

[b8]

(In reply to Magnus Melin [:mkmelin] from comment #6)

Can't reproduce. Maybe bug 1790619 will improve matters.

That's about the dialogue width, this bug here is about the size. Seems to be a timing issue since you need just the "right" message size (2 MB picture in our case) to reproduce it. It only happens once after TB start on an encrypted message. Right now, it was reproducible on first try, maybe the -purgecaches we're using made a difference.

The TypeError mentioned in comment #2 has now morphed to on the current beta:

Uncaught TypeError: document.querySelector(...).shadowRoot is null
    <anonymous> chrome://messenger/content/dialogShadowDom.js:13
    sync chrome://openpgp/content/modules/cryptoAPI/interface.js:56
    encryptMessageStart chrome://openpgp/content/modules/encryption.jsm:437
    finishCryptoEncapsulation chrome://openpgp/content/modules/mimeEncrypt.jsm:524
    createMessageFile resource:///modules/MimeMessage.jsm:87
dialogShadowDom.js:13:12

Comment 9

[b8]

Emilio, what do you think of changing
https://searchfox.org/mozilla-central/rev/8a4f55bc09ffc5c25dcb4586c51ae4a9fee77b4c/toolkit/content/widgets/dialog.js#335
to if (this._l10nButtons?.length) { and addressing
https://searchfox.org/comm-central/rev/2f9e5d0e6bbeb47cd25aa0da8e654c8cec332d9f/mail/base/content/dialogShadowDom.js#13
somehow?

This all seems to be caused by the sync (of an async process) in the PGP code.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Null-checking _l10nButtons seems reasonable. I'm not sure what "addressing https://searchfox.org/comm-central/rev/2f9e5d0e6bbeb47cd25aa0da8e654c8cec332d9f/mail/base/content/dialogShadowDom.js#13" means, can you elaborate?

Comment 11

[b8]

Maybe something like:

window.addEventListener("load", () => {
  let shadowRoot = document.querySelector("dialog").shadowRoot;
  if (shadowRoot) {
    let link = document.createElement("link");
    link.setAttribute("rel", "stylesheet");
    link.setAttribute("href", "chrome://messenger/skin/themeableDialog.css");
    shadowRoot.appendChild(link);
  } else {
    console.error("Can't append stylesheet to shadow root of dialog");
  }
});

Of course this papers over the fact that the shadow root should be set when the load event fires, or not? That would need further investigation.

We assume that the window wouldn't be "themed" of the CSS can't be appended.

Comment 12

[Magnus Melin [:mkmelin]]

(In reply to b8 from comment #11)

Of course this papers over the fact that the shadow root should be set when the load event fires, or not?

See bug 1752288 comment 14, I bet it's the same issue. And in case it is, adding a fluent link to sendProgress.xhtml should work around it.

Comment 13

[b8]

(In reply to Magnus Melin [:mkmelin] from comment #12)

..., adding a fluent link to sendProgress.xhtml should work around it.

Adding a line like this https://hg.mozilla.org/comm-central/rev/5416b7813b53e52b2f23b15b2295e0ea867fe025#l1.12 appears to fix the issue. No TypeError and the dialog is correctly sized.

Comment 14

[Magnus Melin [:mkmelin]]

Attachment: Bug 1777788 - Work around sendProgress.xhtml sizing issues (due to exception) while sending encrypted emails. r=#thunderbird-reviewers

Comment 15

[Kai Engert (:KaiE:)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

Null-checking _l10nButtons seems reasonable. I'm not sure what "addressing https://searchfox.org/comm-central/rev/2f9e5d0e6bbeb47cd25aa0da8e654c8cec332d9f/mail/base/content/dialogShadowDom.js#13" means, can you elaborate?

You didn't do this yet in a patch.

I had reported the same problems 9 months ago in bug 1763826, but then it stopped being a problem for me, and I didn't have time to answer the follow-up question.

And I had suggested the same change as comment 9 here.

So, given you haven't patched this here yet, let me attach a patch in bug 1763826 and ask Emilio for review.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8ec6a98d1397
Work around sendProgress.xhtml sizing issues (due to exception) while sending encrypted emails. r=Paenglab

Comment 17

[Magnus Melin [:mkmelin]]

Comment on attachment 9310768 [details]
Bug 1777788 - Work around sendProgress.xhtml sizing issues (due to exception) while sending encrypted emails. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): bug 1703164
User impact if declined: in some cases the send progress dialog may have wrong size
Testing completed (on c-c, etc.): beta
Risk to taking this patch (and alternatives if risky): safe

Comment 18

[Wayne Mery (:wsmwk)]

Comment on attachment 9310768 [details]
Bug 1777788 - Work around sendProgress.xhtml sizing issues (due to exception) while sending encrypted emails. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr102

Comment 19

[Rob Lemley [:rjl]]

Thunderbird 102.8.0:
https://hg.mozilla.org/releases/comm-esr102/rev/20805bb64e38