Closed Bug 681148 Opened 9 years ago Closed 9 years ago

Define getPostDataStream() as in Firefox

Categories

(SeaMonkey :: General, enhancement)

enhancement
Not set

Tracking

(seamonkey2.4 fixed, seamonkey2.5 fixed)

VERIFIED FIXED
Tracking Status
seamonkey2.4 --- fixed
seamonkey2.5 --- fixed

People

(Reporter: stanio, Assigned: stanio)

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 1 obsolete file)

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
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
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.
Whiteboard: [good first bug]
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.)
Attached patch Initial patch (obsolete) — Splinter Review
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).
Attachment #556276 - Flags: review?(jh)
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!
Attachment #556276 - Flags: review?(jh) → review?(iann_bugzilla)
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.
Attachment #556276 - Flags: review?(iann_bugzilla) → review+
Assignee: nobody → stanio
Status: NEW → ASSIGNED
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.
Attached patch Revised patchSplinter Review
Addresses nit from review.
Attachment #556276 - Attachment is obsolete: true
(In reply to comment #6)

Sorry for the delay.  Now that I've attached an updated patch - should I request another review, or?
Keywords: checkin-needed
(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).
Attachment #556614 - Flags: review?(stanio)
Comment on attachment 556614 [details] [diff] [review]
Revised patch

Per comment #5.
Attachment #556614 - Flags: review?(stanio) → review+
Attachment #556614 - Flags: approval-comm-beta?
Attachment #556614 - Flags: approval-comm-aurora?
Attachment #556614 - Flags: approval-comm-beta?
Attachment #556614 - Flags: approval-comm-beta+
Attachment #556614 - Flags: approval-comm-aurora?
Attachment #556614 - Flags: approval-comm-aurora+
Keywords: checkin-needed
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.