Receive an event with text/Calendar - wouldn't work. So not using lowercase confused mime in two places, making it hard to find what was wrong. To test it with lightning and a file, I made a very simple mozmill test, but had to fix a few cases where lightning assumed msg.folder was set. All the test does is check the bar is really showing. make -C calendar/test/mozmill SOLO_TEST=invitations/test-imip-bar.js mozmill-one

For manual testing

Comment on attachment 8839671 [details] [diff] [review] cal-text-debug.patch Gah, accidentially removed a needed change to imip-bar.js

Correct, working version.

Comment on attachment 8839677 [details] [diff] [review] bug13454379_text_Calendar_imip.patch Review of attachment 8839677 [details] [diff] [review]: ----------------------------------------------------------------- r+ for the mailnews changes. ::: calendar/test/mozmill/invitations/message-containing-event.eml @@ +15,5 @@ > +X-Spam-Status: LOW ; 31% > +X-PMX-Spam: Gauge=XXXI, Probability=31%, Report=' > + AALTO_ENVELOPE+ 1, AALTO_FROM_01+ 0.75, AALTO_KLIKKAA_02+ 0.5, AALTO_SUSP_URL_PARTS+ 0.5, IMGSPAM_BODY 0.5, HTML_70_90 0.1, FROM_NAME_ONE_WORD 0.05, BODYTEXTH_SIZE_10000_LESS 0, BODYTEXTH_SIZE_3000_MORE 0, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_6000_6999 0, BODY_SIZE_7000_LESS 0, DATE_TZ_NA 0, DKIM_SIGNATURE 0, LINK_TO_IMAGE 0, __ANY_URI 0, __C230066_P5 0, __CP_URI_IN_BODY 0, __CT 0, __CTYPE_HAS_BOUNDARY 0, __CTYPE_MULTIPART 0, __CTYPE_MULTIPART_MIXED 0, __FRAUD_MONEY_CURRENCY 0, __FRAUD_MONEY_CURRENCY_EURO 0, __HAS_ATTACHMENT 0, __HAS_ATTACHMENT1 0, __HAS_FROM 0, __HAS_HTML 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HIGHBITS 0, __HTML_AHREF_TAG 0, __HTTPS_URI 0, __HTTP_IMAGE_TAG 0, __IMGSPAM_BODY 0, __MIME_HTML 0, __MIME_VERSION 0, __MULTIPLE_URI_HTML 0, __MULTIPLE_URI_TEXT 0, __SANE_MSGID 0, __TAG_EXISTS_HTML 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_IN_BODY 0, __URI_NO_MAILTO 0, > + __URI_NS , __URI_WITH_PATH 0' > +X-Spam-Flag: No Can you please strip some of those unnecessary headers. ::: calendar/test/mozmill/invitations/test-imip-bar-eml.js @@ +37,5 @@ > + > + msgc.waitFor(function() { > + let nb = msgc.window.document.getElementById("imip-bar"); > + if (!nb) { > + throw new Error("Could'f find imip-bar in DOM."); typo.

Comment on attachment 8839671 [details] [diff] [review] cal-text-debug.patch Review of attachment 8839671 [details] [diff] [review]: ----------------------------------------------------------------- Thanks. r+ for the calendar part with the style nits considered and an at least one additional variant of the mime type like text/Calendar being tested. Please kick of a try run before landing to make sure this test passes on all platforms. ::: calendar/base/modules/calItipUtils.jsm @@ +471,5 @@ > } > > // First check the recipient list > + let toList = MailServices.headerParser.makeFromDisplayAddress( > + aMsgHdr.recipients || ""); Please keep that in one line. @@ +481,5 @@ > } > > // Maybe we are in the CC list? > + let ccList = MailServices.headerParser.makeFromDisplayAddress( > + aMsgHdr.ccList || ""); Same here. ::: calendar/test/mozmill/invitations/message-containing-event.eml @@ +15,5 @@ > +X-Spam-Status: LOW ; 31% > +X-PMX-Spam: Gauge=XXXI, Probability=31%, Report=' > + AALTO_ENVELOPE+ 1, AALTO_FROM_01+ 0.75, AALTO_KLIKKAA_02+ 0.5, AALTO_SUSP_URL_PARTS+ 0.5, IMGSPAM_BODY 0.5, HTML_70_90 0.1, FROM_NAME_ONE_WORD 0.05, BODYTEXTH_SIZE_10000_LESS 0, BODYTEXTH_SIZE_3000_MORE 0, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_6000_6999 0, BODY_SIZE_7000_LESS 0, DATE_TZ_NA 0, DKIM_SIGNATURE 0, LINK_TO_IMAGE 0, __ANY_URI 0, __C230066_P5 0, __CP_URI_IN_BODY 0, __CT 0, __CTYPE_HAS_BOUNDARY 0, __CTYPE_MULTIPART 0, __CTYPE_MULTIPART_MIXED 0, __FRAUD_MONEY_CURRENCY 0, __FRAUD_MONEY_CURRENCY_EURO 0, __HAS_ATTACHMENT 0, __HAS_ATTACHMENT1 0, __HAS_FROM 0, __HAS_HTML 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HIGHBITS 0, __HTML_AHREF_TAG 0, __HTTPS_URI 0, __HTTP_IMAGE_TAG 0, __IMGSPAM_BODY 0, __MIME_HTML 0, __MIME_VERSION 0, __MULTIPLE_URI_HTML 0, __MULTIPLE_URI_TEXT 0, __SANE_MSGID 0, __TAG_EXISTS_HTML 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_IN_BODY 0, __URI_NO_MAILTO 0, > + __URI_NS , __URI_WITH_PATH 0' > +X-Spam-Flag: No I think you can spare the X- headers here. @@ +39,5 @@ > + > +--_=ALT_=aspNetEmail=_51bed191ceac49f7a22392ea84b6ef35-- > + > +--_=aspNetEmail=_51bed191ceac49f7a22392ea84b6ef35 > +Content-Type: text/calendar; name="booking.ics" As the bug is about being able to deal also with not all lowercase mime type, you probably want to additionally test another variant like text/Calendar Apart from that, this is not an RfC (6047) compliant specification of Content-Type - this should be Content-Type: text/calendar; method=PUBLISH; charset=UTF-8 (with method being mandatory and charset mandatory if non UTF-8 characters would occur - specifying a name is not needed here) ::: calendar/test/mozmill/invitations/test-imip-bar-eml.js @@ +12,5 @@ > + > +var RELATIVE_ROOT = "../shared-modules"; > +var MODULE_REQUIRES = [ > + "folder-display-helpers", > + "window-helpers", Unlike TB, Calendar uses 4 whitespace indentation, so please adjust this everywhere in the new test. @@ +34,5 @@ > + let file = os.getFileForPath(os.abspath("./message-containing-event.eml", thisFilePath)); > + > + let msgc = open_message_from_file(file); > + > + msgc.waitFor(function() { Please make this an arrow function: msgc.waitFor(() => { @@ +37,5 @@ > + > + msgc.waitFor(function() { > + let nb = msgc.window.document.getElementById("imip-bar"); > + if (!nb) { > + throw new Error("Could'f find imip-bar in DOM."); Typo. @@ +38,5 @@ > + msgc.waitFor(function() { > + let nb = msgc.window.document.getElementById("imip-bar"); > + if (!nb) { > + throw new Error("Could'f find imip-bar in DOM."); > + } We usually use some helpers from calendar-utils for looking up the DOM element. If you want to get the bonus points, look at the other calendar tests an pick up the approach. But as this test will get touched anyway when landing my WIP patch for mozmill invitation tests, you also can leave it with your current approach for now.

Addressed comments. https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=14744771d5e0beae710b1f7d7c33a5eb1e7e1774

Comment on attachment 8840072 [details] [diff] [review] bug13454379_text_Calendar_imip.patch [Approval Request Comment] User impact if declined: events with non-lowercase mime type can't be added to calendar (in any easy way) Testing completed (on c-c, etc.): mozmill test Risk to taking this patch (and alternatives if risky): not risky

Comment on attachment 8840072 [details] [diff] [review] bug13454379_text_Calendar_imip.patch I can't do Aurora, since it's already *in* Aurora, TB 54. We don't uplift to c-r. I can do c-ers52 after a beta run, gotta stick to the rules.

Beta (TB 53): https://hg.mozilla.org/releases/comm-beta/rev/b30af93360d01f9072fafb5d3902296462fef77b

TB 52 ESR: https://hg.mozilla.org/releases/comm-esr52/rev/72a7fd03d6be7f26f453e3fe76e6209fbc12d4c1 I've just noticed that the commit message for all branches has non-existent bug 13454379 :-(

I backed that out and landed it again, at least on C-C so we stand the chance to track the changes back to bug: https://hg.mozilla.org/comm-central/rev/19dc916b9af963c59a54b11d7208dbb89e819a25 https://hg.mozilla.org/comm-central/rev/7480da62b6bfede9e4a5fe401da22989865ec73b While doing so I've looked at it again and noticed: https://hg.mozilla.org/comm-central/rev/7480da62b6bfede9e4a5fe401da22989865ec73b#l2.1 copy from calendar/lightning/content/imip-bar.js copy to calendar/lightning/content/imip-bar-eml.js Why was that file copied? Both files then received identical edits. And imip-bar-eml.js doesn't appear to be used anywhere.

That was not supposed to be there. I had accidentally renamed/moved one file, but apparently didn't get it cleared up properly. https://hg.mozilla.org/comm-central/rev/d68d335c34a7f42fadda05c06db127b1f264ff5c

Thanks. Removed from beta and from esr52 as well for good housekeeping: https://hg.mozilla.org/releases/comm-beta/rev/567c3434053e258ca976b3362a5d2c40a2b0f8b2 https://hg.mozilla.org/releases/comm-esr52/rev/14a25e8ce0bc4d13d118a1183041d3fe88fcf031