Closed Bug 1200263 Opened 4 years ago Closed 4 years ago

'Help → Report Web Forgery ...' does nothing

Categories

(SeaMonkey :: Security, defect)

defect
Not set

Tracking

(seamonkey2.38 wontfix, seamonkey2.39 wontfix, seamonkey2.41 wontfix, seamonkey2.42 fixed, seamonkey2.43 fixed, seamonkey2.44 fixed)

RESOLVED FIXED
seamonkey2.44
Tracking Status
seamonkey2.38 --- wontfix
seamonkey2.39 --- wontfix
seamonkey2.41 --- wontfix
seamonkey2.42 --- fixed
seamonkey2.43 --- fixed
seamonkey2.44 --- fixed

People

(Reporter: thee.chicago.wolf, Assigned: frg)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0 SeaMonkey/2.38
Build ID: 20150831085605

Steps to reproduce:

Clicked Help > Report Web Forgery.


Actual results:

It did not take me to the report phishing page site.


Expected results:

It should take me to the report phishing page site. Works in 2.37. Tested with https://l10n.mozilla-community.org/~akalla/unofficial/seamonkey/nightly/ Tested with 2.38.
Still busted in 2.39 20151021030937.
REPRODUCIBLE with  en-US (German language pack) SeaMonkey 2.38  (Windows NT 6.1; WOW64; rv:41.0)  Gecko/20100101 Firefox/41.0 Build 20150923195647  (Classic Theme) on German WIN7 64bit

Additional Info:
a) What it should do you find here <https://wiki.mozilla.org/SeaMonkey/Help#Report_Web_Forgery_...>
b) Works fine with FF 44.0a1 (2015-10-28)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Report Web Forgery function broken → 'Help → Report Web Forgery ...' does nothing
e) Error console tells
   Timestamp: 01.11.2015 10:04:58
   Error: TypeError: URI is undefined
   Source File: resource://gre/modules/SafeBrowsing.jsm
   Line: 114
c) Regression, worked fine with "SeaMonkey 2.35 (Windows NT 6.1; WOW64; rv:38.0)
   Gecko/20100101 Build 20150827182544 (Classic Theme) on German WIN7 64bit
d) currently only reproduced with WIN
Keywords: regression
OS: Unspecified → Windows 7
d): Also REPRODUCIBLE with  German SeaMonkey 2.38  (X11; Linux s86_64; rv:41.0)
    Gecko/20100101 Firefox/41.0 Build 20150923193515  (Classic Theme) 
    on VirtualBox Ubuntu 14
OS: Windows 7 → All
e) Still REPRODUCIBLE with  German SeaMonkey 2.41a1 Mozilla/5.0 (Windows NT 6.1; 
   Win64; x64; rv:44.0 from akalla download area)  Gecko/20100101  Firefox/ 44.0
   Build 20150926114543, (Classic Theme) on German WIN7 64bit

f) Already REPRODUCIBLE with  SeaMonkey 2.38a1  (Windows NT 6.1; WOW64; rv:41.0 
   nightly by Philip Chee)  Gecko/20100101 Firefox/41.0 Build 20150620233542  
   (Classic Theme) on German WIN7 64bit

c): Was still ok with  SeaMonkey 2.38a1  (Windows NT 6.1; WOW64; rv:41.0 nightly
    from unknown Source)  Gecko/20100101 Firefox/41.0 Build 20150523065810 
    (Classic Theme) on German WIN7 64bit
Attached patch 1200263-reporturl.patch (obsolete) — Splinter Review
Reporturl function changed. Now takes an URI as the second parameter.
Attachment #8700406 - Flags: review?(bugspam.Callek)
Attached patch 1200263-reporturl-V2.patch (obsolete) — Splinter Review
I copied the wrong patch out of my directory. It still contained an error. URI.clone would fail with the old one in SafeBrowsing.jsm.
Attachment #8700406 - Attachment is obsolete: true
Attachment #8700406 - Flags: review?(bugspam.Callek)
Attachment #8702083 - Flags: review?(bugspam.Callek)
Hardware: Unspecified → All
Version: SeaMonkey 2.38 Branch → Trunk
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Component: General → Help
Comment on attachment 8702083 [details] [diff] [review]
1200263-reporturl-V2.patch

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

redirect review to :Ratty, I won't be able to get to this too soon
Attachment #8702083 - Flags: review?(bugspam.Callek) → review?(philip.chee)
This bug is blocking Bug 1220707.  Could someone with the requisite permissions please mark it as such?
Blocks: 1220707
Comment on attachment 8702083 [details] [diff] [review]
1200263-reporturl-V2.patch

>      // Remove the query to avoid including potentially sensitive data
>      if (pageUri instanceof Components.interfaces.nsIURL)
>        pageUri.query = "";
Remove this chunk as well since SafeBrowsing.getReportURL() already does this.

r=me with the above fixed.
Attachment #8702083 - Flags: review?(philip.chee) → review+
Revised patch with review+ from Philip Chee carried forward.
Attachment #8702083 - Attachment is obsolete: true
Attachment #8722191 - Flags: review+
Keywords: checkin-needed
> -    var reportUrl = SafeBrowsing.getReportURL(aName);
.......
> +    reportUrl = SafeBrowsing.getReportURL(aName, pageUri);
> +
>      return reportUrl;

You removed the var so you need to reinsert it:
    var reportUrl = SafeBrowsing.getReportURL(aName, pageUri);
    return reportUrl;

Or even simpler:
    return SafeBrowsing.getReportURL(aName, pageUri);

I'll fix this on checkin.
Thanks. You are right. I didn't catch it. Wonder why it worked. Just tested again without the var.

FRG
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/afa5af5768bd

I also added the comment change from Bug 920951 so you should remove the whole hunk over there.
> Thanks. You are right. I didn't catch it. Wonder why it worked. Just tested
> again without the var.

Without the var it becomes a global var and gets hoisted to the top of the file.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Thanks. Still learning:)

Will redo the patch in Bug 920951 anyway to put the comments in and start with a clean base.

FRG
Comment on attachment 8722191 [details] [diff] [review]
Patch to fix  getReportURL function

[Approval Request Comment]
Regression caused by (bug #): various....

User impact if declined:
Click Help > Report Web Forgery. Does not take the user to the report phishing page site.

Testing completed (on m-c, etc.): comm-central

Risk to taking this patch (and alternatives if risky): none - bustage fix.

String changes made by this patch:
Attachment #8722191 - Flags: approval-comm-beta?
Attachment #8722191 - Flags: approval-comm-aurora?
Component: Help Documentation → Security
Comment on attachment 8722191 [details] [diff] [review]
Patch to fix  getReportURL function

Switching branch requests to reflect 2.44 to 2.45 merges.
Attachment #8722191 - Flags: approval-comm-aurora? → approval-comm-release?
Comment on attachment 8722191 [details] [diff] [review]
Patch to fix  getReportURL function

a=me for c-b/c-r/c-esr
Attachment #8722191 - Flags: approval-comm-release?
Attachment #8722191 - Flags: approval-comm-release+
Attachment #8722191 - Flags: approval-comm-beta?
Attachment #8722191 - Flags: approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.