681148 - Define getPostDataStream() as in Firefox

Status: VERIFIED FIXED

(SeaMonkey :: General, enhancement)

Description

[Stanimir Stamenkov]

Firefox defines a getPostDataStream() function in its <chrome://browser/content/browser.js> which appears reused by extensions like Secure Login [1].  A (not yet fully) SeaMonkey compatible version [2] of that extension has been made available on the xSidebar site.  That version currently fails to work properly if its "JavaScript protection on login" feature is activated:

ReferenceError: getPostDataStream is not defined

Although, I don't have statistics if this function is used by lots of extensions or just extensions with larger user base, I guess it would be nice if getPostDataStream() is made available in SeaMonkey for better Firefox compatibility in terms of extension support.

Justin Wood (Callek) suggested in the mozilla.dev.apps.seamonkey group, this function was a missed port by Bug 580660.

[1] https://addons.mozilla.org/addon/secure-login/
[2] http://xsidebar.mozdev.org/modifiedmisc.html#securelogin

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Stanimir, would you like to try and get your hands dirty, i.e. provide a patch? We can surely guide you through.

Basically all that needs to be done is this:
0. get a copy of comm-central (cf. https://wiki.mozilla.org/SeaMonkey/Good_First_Bugs)
1. copy the function from browser.js to navigator.js
2. adapt the function by replacing "Cc" by "Components.classes" and "Ci" by "Components.interfaces"
3. line up dots like .classes one one line and .createInstance on the next
4. create a patch using hg diff
5. upload the patch and request review here.

Comment 2

[Tony Mechelynck [:tonymec]]

Yes, and see also:
- https://developer.mozilla.org/En/Developer_Guide/Build_Instructions
- http://www.mozilla.org/developer/
- https://developer.mozilla.org/en-US/search?q=Mercurial
(as well as what they link to) and note that the preferred diff format on Mozilla has 8 lines of unified context. (Yes, that's a lot.)

Comment 3

[Stanimir Stamenkov]

Attachment: Initial patch

Initial untested patch, until I get my first full SeaMonkey build done.

Please verify I've got the style correct (point 3 from comment 1).

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 556276 [details] [diff] [review]
Initial patch

Hehe, I feel honored, but I'm not (an accepted) code reviewer. Looking at MXR's file log for navigator.js, possible candidates are Ian and Neil. Choosing the former just so that people can't say I'm always choosing Neil. ;-)

Other than that, looks good!

Comment 5

[Ian Neal]

Comment on attachment 556276 [details] [diff] [review]
Initial patch

>+function getPostDataStream(aStringData, aKeyword, aEncKeyword, aType) {
Nit: { on new line please, to match the other functions in this file.

> function handleDroppedLink(event, url, name)
> {
r=me with that fixed.

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

Stanimir, I'll explain to you how our process works below. In the future I or others might skip some steps and just check in whatever is ready, but since you cannot rely on that, following the process helps us to not miss anything.

If you provide a patch and get a positive review, you become the de facto assignee of the bug. In this case, Ian already made that "official" by setting the corresponding status and flag, but this is really something anyone can do who provides a patch (or even plans to do so!).

The next step is to get it checked in. Since you don't have the rights to do it yourself, our process asks you (the assignee) to set the "checkin-needed" keyword. Then someone with appropriate rights will drive the patch in, taking care of properly attributing you for your contribution, and either resolve the bug if it's apparently done, or leave that to you (the assignee). Usually the one who checks in will also set the Target Milestone to the correct value (current trunk version).

After that the fix should be verified using the next nightly build that contains the fix, and the bug status be updated.

Then, optionally, branch approval may be requested for high benefit/low risk patches that improve stability or security, or maybe even compatibility improvements like this one that doesn't affect existing (application) code. Requesting approval is done on individual attachments by setting the approval‑comm‑aurora/beta flags to "?". If a request is granted, set checkin-needed again and include in the Whiteboard field to which branch(es) it applies. Usually the one who checks in will then set the Tracking Flags to their correct values.

Comment 7

[Stanimir Stamenkov]

Attachment: Revised patch

Addresses nit from review.

Comment 8

[Stanimir Stamenkov]

(In reply to comment #6)

Sorry for the delay.  Now that I've attached an updated patch - should I request another review, or?

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Stanimir Stamenkov from comment #8)
> Sorry for the delay.  Now that I've attached an updated patch - should I
> request another review, or?

No, just set r+ yourself on your latest patch (since Ian said "r=me with that fixed" in comment 5).

Comment 10

[Stanimir Stamenkov]

Comment on attachment 556614 [details] [diff] [review]
Revised patch

Per comment #5.

Comment 11

[Philip Chee]

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/38399c45a2a8

Comment 12

[Philip Chee]

Pushed:
http://hg.mozilla.org/releases/comm-aurora/rev/ceb61068f6e7
http://hg.mozilla.org/releases/comm-beta/rev/3bb1aadc6f8e