1368669 - runtime.sendMessage() argument parsing should consider an undefined callback

Status: VERIFIED FIXED

(WebExtensions :: General, defect)

Description

[Vasilica Tamas, QA [:vtamas]]

Attachment: Animation.gif

[Affected versions]:
Firefox 55.0a1 (2017-05-29)
 
[Affected platforms]:
Windows 10 64-bit
Ubuntu 16.04 32-bit
 
[Steps to reproduce]:
1.Launch Firefox with a clean profile.
2.Install the following add-on: https://addons.mozilla.org/en-US/firefox/addon/screenshot-capture-annotate/
3.Navigate to a new webpage (e.g. https://www.amazon.com/)
4.Click on the webextension icon from toolbar.
5.Select “Capture the entire page” or “Capture selected area”.
 
 
[Expected Results]:
The webextension options work without any issue.
 
 
[Actual Results]:
- The screenshot is not opened in webextension editing tab.
- The following errors are thrown in Browser Console while selecting the affected capturing options: 
SyntaxError: expected expression, got ')'  void();:1:5
Error: runtime.sendMessage's extensionId argument is invalid  undefined
- See attached screencast.  
 
 
[Regression Range]:
Last good revision: 952c5438a347b382be5f6ac516365c9a299818bd
First bad revision: 70db5f9c852d04d2d73e879f80385c2c484c2f17
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=952c5438a347b382be5f6ac516365c9a299818bd&tochange=70db5f9c852d04d2d73e879f80385c2c484c2f17
 
Looks like the following bug has the  changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1259944

Comment 1

[Andrew Swan [:aswan]]

Ugh, Awesome Screenshot is calling runtime.sendMessage(object, undefined) with the intention that object is the message and undefined is the callback.  But based on [1] we treat those arguments as extensionId, message.

[1] http://searchfox.org/mozilla-central/rev/b318c7dca7392bd16c0b11929f55b1be133f0b31/toolkit/components/extensions/ext-c-runtime.js#57-67

Comment 2

[:shell escalante]

andy said you recently changed this area

Comment 3

[Tomislav Jovanovic :zombie]

Attachment: Bug 1368669 - Support explicit null callback for runtime.sendMessage

Review commit: https://reviewboard.mozilla.org/r/146916/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146916/

Comment 4

[Andrew Swan [:aswan]]

Comment on attachment 8875478 [details]
Bug 1368669 - Support explicit null callback for runtime.sendMessage

https://reviewboard.mozilla.org/r/146916/#review151522

Thanks, I was overthinking this, thinking it would be more complicated but this looks good.

Comment 5

[Tomislav Jovanovic :zombie]

Comment on attachment 8875478 [details]
Bug 1368669 - Support explicit null callback for runtime.sendMessage

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/146916/diff/1-2/

Comment 6

[Tomislav Jovanovic :zombie]

Comment on attachment 8875478 [details]
Bug 1368669 - Support explicit null callback for runtime.sendMessage

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/146916/diff/2-3/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c89f7b976dea
Support explicit null callback for runtime.sendMessage r=aswan

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c89f7b976dea

Comment 9

[Vasilica Tamas, QA [:vtamas]]

Confirm that this issue is fixed on Firefox 55.0a1 (2017-06-11) under Windows 10 64-bit and Ubuntu 16.04 32-bit. The webextension options work as expected.

Comment 10

[Will Bamberg [:wbamberg]]

Updated (again!) -> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/sendMessage#Parameters

Let me know if this is accurate.

Comment 11

[Tomislav Jovanovic :zombie]

I have reparse this every time, but yeah, it looks good.