682183 - compose window is not noticing attachment keywords

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Jonathan Kamens]

Some time in the last few days comm-central broke so Thunderbird is no longer prompting me about potentially missing attachments when I use one of the attachment keywords in my message text.

Comment 1

[Jonathan Kamens]

It broke more than a few days ago, actually. A search of the nightly builds reveals that it broke between 2011-07-25-03-00-33 and 2011-07-26-03-00-25. That means it's broken in the Aurora channel. Need to get it fixed before it gets released!

Comment 3

[Jason Yeo[:jyeo]]

Found out that it was mozilla-central code that caused the regression

M-C change set
Tue Jul 26 02:50:30 2011 -0700 f631e1b0e296
breaks

Tue Jul 26 02:50:30 2011 -0700 1d3b050ee50a
works!

Suspect it's some new web worker code that caused it. The attachmentWorker.onmessage function in MsgMailCommands.js is never triggered.

Comment 4

[Jason Yeo[:jyeo]]

Attachment: bug682183 patch

Comment 5

[Rain [:rain1] (inactive)]

Does this actually fix the issue? There shouldn't be a difference between function onmessage(event) and onmessage = function(event) unless I'm missing something.

Comment 6

[Rain [:rain1] (inactive)]

Hm, seems like onmessage is a little special. Never mind.

Comment 7

[Jason Yeo[:jyeo]]

yup it's kinda special. see https://developer.mozilla.org/en/Using_web_workers

it says...
The worker sets the property onmessage to a function which will receive messages sent when the worker object's postMessage() is called.  (Note that this differs from defining a global variable of that name, or defining a function with that name.  var onmessage and function onmessage will define global properties with those names, but they will not register the function to receive messages sent by the  web page that created the worker.)

Comment 8

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 559615 [details] [diff] [review]
bug682183 patch

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

It's small, it's simple, it fixes the problem.  I like it!

The only thing I might ask for is a comment telling us that we can't make that "function onmessage" or "var onmessage", as per the comment in the url, but I won't let that block this from landing.

And I'm asking for approval on aurora, and possibly beta if we don't land it by the time we branch.  ;)

Thanks,
Blake.

Comment 9

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 559615 [details] [diff] [review]
bug682183 patch

Yeah, thanks for the heads-up, IanN...

Comment 10

[Jonathan Protzenko [:protz]]

http://hg.mozilla.org/comm-central/rev/71a96aa84eb5

Comment 11

[Mark Banner (:standard8)]

Transplanted to aurora: http://hg.mozilla.org/releases/comm-aurora/rev/bed118c12fa5