Test "browser_sanitize-download-history.js" contains typo in window name

RESOLVED FIXED in mozilla22

Status

()

Toolkit
Downloads API
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Mic, Assigned: bkerensa)

Tracking

Trunk
mozilla22
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
The test at http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/browser_sanitize-download-history.js contains a check [1] that should close the UI if it's already open. The window's name is given with "Sanatize" and that seems to be a typo that breaks this part (it's the only reference to "Sanatize" that MXR can find!) and should read "Sanitize" instead.

[1] http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/browser_sanitize-download-history.js#109 (below "// Close the UI if necessary" if the line numbers should get messed up).
(Reporter)

Comment 1

5 years ago
I hope setting "Blocks 431729" is correct, since it was introduced with this bug as it seems?
Blocks: 431729
(Assignee)

Comment 2

5 years ago
Created attachment 641555 [details] [diff] [review]
Fix the file

This should fix it.
(Assignee)

Updated

5 years ago
Attachment #641555 - Flags: review?(dietrich)
Comment on attachment 641555 [details] [diff] [review]
Fix the file

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

while the change is correct, the patch format is not, please check https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Attachment #641555 - Flags: review?(dietrich) → review-
Assignee: nobody → bkerensa
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 641555 [details] [diff] [review]
Fix the file

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

ehr, for some reason bugzilla showed me something else, sorry :)

Btw, if you could add user and commit message it would be welcome, then just add checkin-needed keyword
Attachment #641555 - Flags: review- → review+
(Assignee)

Comment 5

5 years ago
I already nuked that branch what specific strings do you need added to the patch for it to be applied?
(Reporter)

Comment 6

5 years ago
Created attachment 723230 [details] [diff] [review]
Patch with header (commit message, user,..)

> Btw, if you could add user and commit message it would be welcome, then just
> add checkin-needed keyword

I created a new patch, including a username and commit message. I couldn't find any previous patchs and checkins from you though, Benjamin Kerensa. If you'd like to have your name and email adress of your choice there, then replace mine and add the checkin-needed keyword to the whiteboard.

The patch is against mozilla-central and I had updated the repository right before exporting it. If something is wrong, please let me know. It's the first patch I created for m-c.
(Assignee)

Comment 7

5 years ago
Comment on attachment 723230 [details] [diff] [review]
Patch with header (commit message, user,..)

># HG changeset patch
># User Benjamin Kerensa <bkerensa@ubuntu.com>
># Date 1362941807 -3600
># Node ID 04cebce82099b05a46fc824adf91cbc91c2056a4
># Parent  b1a08130fae67ee3c02c63349628f5b645030cc1
>Bug 758874 - Test browser_sanitize-download-history.js contains typo in window name; r=mak77
>
>diff --git a/browser/base/content/test/browser_sanitize-download-history.js b/browser/base/content/test/browser_sanitize-download-history.js
>--- a/browser/base/content/test/browser_sanitize-download-history.js
>+++ b/browser/base/content/test/browser_sanitize-download-history.js
>@@ -103,17 +103,17 @@ function test()
>   let dm = Cc["@mozilla.org/download-manager;1"].
>            getService(Ci.nsIDownloadManager);
>   let db = dm.DBConnection;
> 
>   // Empty any old downloads
>   db.executeSimpleSQL("DELETE FROM moz_downloads");
> 
>   // Close the UI if necessary
>-  let win = Services.ww.getWindowByName("Sanatize", null);
>+  let win = Services.ww.getWindowByName("Sanitize", null);
>   if (win && (win instanceof Ci.nsIDOMWindow))
>     win.close();
> 
>   // Start the test when the sanitize window loads
>   Services.ww.registerNotification(function (aSubject, aTopic, aData) {
>     Services.ww.unregisterNotification(arguments.callee);
>     aSubject.QueryInterface(Ci.nsIDOMEventTarget)
>             .addEventListener("DOMContentLoaded", doTest, false);
(Assignee)

Updated

5 years ago
Whiteboard: checkin-needed
(Reporter)

Comment 8

5 years ago
Created attachment 723378 [details] [diff] [review]
Patch with header (commit message, user,..)

This patch is attachment 641555 [details] [diff] [review] with headers and Benjamin Kerensa's user information now. I assume this carries mak77's r+ forward? I would have obsoleted the old patch but I haven't got sufficient rights to do so.


I'm not super-familiar with the checkin-process here but pasting an edited patch in a comment didn't look useful to me. When I wrote "replace" I had expected that you would download, edit and re-upload the diff I created for you.

Thanks for providing your user information!
Attachment #723230 - Attachment is obsolete: true
(Reporter)

Updated

5 years ago
Keywords: checkin-needed
Whiteboard: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5448e182c48
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b5448e182c48
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.