Define getPostDataStream() as in Firefox

VERIFIED FIXED

Status

--
enhancement
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: stanio, Assigned: stanio)

Tracking

SeaMonkey Tracking Flags

(seamonkey2.4 fixed, seamonkey2.5 fixed)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
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

Updated

7 years ago
Status: UNCONFIRMED → NEW
tracking-seamonkey2.4: --- → ?
tracking-seamonkey2.5: --- → ?
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.)
(Assignee)

Comment 3

7 years ago
Created attachment 556276 [details] [diff] [review]
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).
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 5

7 years ago
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+

Updated

7 years ago
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.
(Assignee)

Comment 7

7 years ago
Created attachment 556614 [details] [diff] [review]
Revised patch

Addresses nit from review.
Attachment #556276 - Attachment is obsolete: true
(Assignee)

Comment 8

7 years ago
(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).
(Assignee)

Updated

7 years ago
Attachment #556614 - Flags: review?(stanio)
(Assignee)

Comment 10

7 years ago
Comment on attachment 556614 [details] [diff] [review]
Revised patch

Per comment #5.
Attachment #556614 - Flags: review?(stanio) → review+
(Assignee)

Updated

7 years ago
Attachment #556614 - Flags: approval-comm-beta?
Attachment #556614 - Flags: approval-comm-aurora?

Updated

7 years ago
Attachment #556614 - Flags: approval-comm-beta?
Attachment #556614 - Flags: approval-comm-beta+
Attachment #556614 - Flags: approval-comm-aurora?
Attachment #556614 - Flags: approval-comm-aurora+

Comment 12

7 years ago
Pushed:
http://hg.mozilla.org/releases/comm-aurora/rev/ceb61068f6e7
http://hg.mozilla.org/releases/comm-beta/rev/3bb1aadc6f8e
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
status-seamonkey2.4: --- → fixed
status-seamonkey2.5: --- → fixed
Resolution: --- → FIXED

Updated

7 years ago
Keywords: checkin-needed
tracking-seamonkey2.4: ? → ---
tracking-seamonkey2.5: ? → ---

Updated

7 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.