1695039 - invites do usually not show as styled anymore

Status: RESOLVED FIXED

(Calendar :: E-mail based Scheduling (iTIP/iMIP), defect, P1)

Description

[Magnus Melin [:mkmelin]]

Likely broken by tb e10s.
Invites in email don't show styled, usually. It's not 100% consistant. Some time you can come back to view the mail, and then the invite is styled as it should. After it starts showing it always shows for that mail I think.

I think it's related to this error in the console:

stylesheets actor: fetch failed for chrome://calendar/content/calendar-event-dialog.css, using system principal instead. style-sheet.js:149:13
fetchStylesheet resource://devtools/server/actors/style-sheet.js:149

Comment 1

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

I don't see the console error, but I am also missing the styling for invites.

I noticed that disabling the invite's corresponding calendar and restarting fixes this.

Moreover, I noticed that previous invites from a modified event no longer show the changes. From https://phabricator.services.mozilla.com/D111411#3674766

I don't actually see the modified document [from ltn.invitation.compareInvitationOverlay] when I open an actual invitation in TB. Instead I see the original one, but without any styling.
...
I personally suspect this line: https://searchfox.org/comm-central/rev/7ed718c90a58141a74da3acbf3f97ad8854e5561/calendar/lightning/content/imip-bar.js#325, because it is only called if there is an associated calendar for the email (the ltnImipBar.foundItems condition at the top of the method).

Comment 2

[Magnus Melin [:mkmelin]]

This seems to have gone permanent now.
Interesting though that the second css file Henry added is applied, but the other one not. (You get the icons, but not colors and alignments etc.)

Comment 3

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

I think the problem may be that imported stylesheets (through @import url(...);) are being stripped.

Here is a summary of the document.styleSheets for the document of the message body:

[
  {
    href: "chrome://messagebody/skin/messageBody.css",
    rules: [
      /* CSSImportRule chrome://communicator/skin/smileys.css */ ... /* Empty imported rules. */,
      /* CSSImportRule chrome://messenger/skin/messageQuotes.css */ ... /* Empty imported rules. */,
      /* Non-imported rules. */
      /* CSSNamespaceRule */ ...,
      ...
    ],
  },
  {
    href: "chrome://messagebody/skin/imip.css",
    rules: [
      {
        /* CSSImportRule */
        stylesheet: {
          href: "chrome://calendar/skin/shared/imip.css",
          /* Empty rules on import! */
          rules: [],
        },
      },
    ],
  },
  {
    href: "chrome://messagebody/skin/calendar-attendees.css",
    /* non-empty */
    rules: [
      /* CSSNamespaceRule */ ...,
      /* CSSStyleRule "html|input.textbox-addressingWidget" */ ...,
      /* CSSStyleRule "html|input.textbox-addressingWidget:disabled" */ ...,
      ...
    ],
  },
]

Note that the import rules for smileys.css, messageQuotes.css and shared/imip.css are all empty. calendar-attendees.css is shared by all platforms, so has no @import rule. Could it be a security policy blocking the import because they are not under chrome://messagebody?

Not sure why calendar-attendees.css didn't work before but does now, could it be that it used to use an import rule until recently?

Also, like I mentioned in the comment#1, if you disable the corresponding calendar and restart Thunderbird, you get the normal styling. In that case the imported rules are present.

Like I said before, I suspect displayHTMLInMessagePane. The current process is basically:

1. Whether the calendar is enabled or not, the raw invite is created in CalMimeConverter.convertToHTML (https://searchfox.org/comm-central/rev/bd3e8b590aab93de5f78d83a2ae71d37cead9ee5/calendar/base/src/CalMimeConverter.jsm#65). This is called by some message parser in cpp. I'm not sure where this call originates from, but it doesn't use msgWindow.displayHTMLInMessagePane.
2. Once that is loaded, if the calendar is enabled, msgWindow.displayHTMLInMessagePane (https://searchfox.org/comm-central/rev/bd3e8b590aab93de5f78d83a2ae71d37cead9ee5/calendar/base/content/imip-bar.js#316) is called to replace the raw invite in the message body with a modified version where modifications to the event are shown. If you comment out this line, then you will see the raw invite instead.

We could try and figure out what is going on, or we could get around it. A way around this could be to pass to compareInvitationOverlay (https://searchfox.org/comm-central/source/calendar/base/content/imip-bar.js#309) the actual message document (from document.getElementById("messagepane").contentDocument) and it can edit it directly, which would save the displayHTMLInMessagePane call. It would also save some processing because the replacement process currently goes through the conversion Document -> XHTML String -> Document -> XHTML String -> Document.

Comment 4

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

Attachment: Bug 1695039 - Show changes to an invitation email by editing the Document directly rather than replacing it. r=darktrojan

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e8b368b2083e
Show changes to an invitation email by editing the Document directly rather than replacing it. r=darktrojan

Comment 6

[Uplift Request]

Please request uplift to TB 91 beta.

Comment 7

[Geoff Lankow (:darktrojan)]

Comment on attachment 9232821 [details]
Bug 1695039 - Show changes to an invitation email by editing the Document directly rather than replacing it. r=darktrojan

[Approval Request Comment]
Regression caused by (bug #): Likely E10s, but also this fixes bug 1723116 which was caused bug 1679299.
User impact if declined: In the original problem, details of invitations are not styled correctly. Currently the details of invitations are not displayed. There is a simpler way to fix the latter but we might as well take this instead.
Testing completed (on c-c, etc.): Landed last week.
Risk to taking this patch (and alternatives if risky): Hard to say, it's a bit convoluted so there's a few possible points of failure. But it works, and this feature is well used so further regressions should be reported quickly.

Comment 8

[Wayne Mery (:wsmwk)]

Comment on attachment 9232821 [details]
Bug 1695039 - Show changes to an invitation email by editing the Document directly rather than replacing it. r=darktrojan

[Triage Comment]
Approved for beta
Hopefully the tests have us well covered.

Comment 9

[Uplift Request]

(In reply to Wayne Mery (:wsmwk) from comment #8)

Hopefully the tests have us well covered.

There are (apparently) no tests that check that the details are displayed, otherwise bug 1723116 would have been detected on the comm-beta treeherder. This bug here modifies test_invitationutils.js, but that's a (headless) unit test that can't check the display either.

Comment 10

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

(In reply to Uplift Request from comment #9)

There are (apparently) no tests that check that the details are displayed, otherwise bug 1723116 would have been detected on the comm-beta treeherder. This bug here modifies test_invitationutils.js, but that's a (headless) unit test that can't check the display either.

This is correct, there are two unit tests: one that tests plain invitations, and one that tests updated invitations. This bug here is about how CSS styling was missing from the updated invitations. I'm not sure it makes sense to have a test for styling rules being present.

But for bug 1723116, it might make sense to add a mochitest for the invite being displayed in the #messagepane. Note that there is different behaviour when the invitation's event is linked to an enabled calendar or not. Moreover, if there is a corresponding calendar, then the behaviour is different if the invitation is newer, older or the same as the version of the event found in the calendar. However, I don't know how to set up these different conditions for such a mochitest.

Comment 11

[Rob Lemley [:rjl]]

Thunderbird 91.0b6:
https://hg.mozilla.org/releases/comm-beta/rev/c23b2abcc627

Comment 12

[José M. Muñoz]

Henry, this fixes bug 1360155, correct?

Comment 13

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

(In reply to José M. Muñoz from comment #12)

Henry, this fixes bug 1360155, correct?

Looking briefly at the other bug, and given that the issue is now fixed, I wouldn't be surprised if the cause of the problem was indeed this line that was removed (https://hg.mozilla.org/comm-central/rev/e8b368b2083e#l1.225) since it is linked to whether a corresponding event is found in the calendar. This comment says the same sort of thing https://bugzilla.mozilla.org/show_bug.cgi?id=1360155#c27

The current approach is to edit the shadow DOM in-place, rather than replace it. I didn't realise a message could display both an invite plus other parts, but that makes sense. But luckily the new editing method works for multipart messages because everything is edited by id.