Clean up non capitalized constants, purge gTimeout,gDelay

RESOLVED FIXED

Status

--
trivial
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jfrench, Assigned: jfrench)

Tracking

unspecified

Firefox Tracking Flags

(firefox19 fixed, firefox20 fixed, firefox21 fixed, firefox22 fixed, firefox-esr17 fixed)

Details

(Whiteboard: [mentor=andreeamatei][lang=js][good first bug], URL)

Attachments

(7 attachments, 3 obsolete attachments)

46.04 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
28.44 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
48.24 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
2.86 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
48.32 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
52.23 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
53.17 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
During other work I noticed a few un-capitalized or otherwise unconventional constant names, in files in the repo. I searched with a little regex to find all the files in the repo that aren't at a minimum all capitalized, or are interCapped.

They are:

tests/functional/testAwesomeBar/testLocationBarSearches.js
tests/functional/testInstallation/testBreakpadInstalled.js
tests/functional/testLayout/testNavigateFTP.js
tests/functional/testPopups/testPopupsAllowed.js
tests/functional/testPopups/testPopupsBlocked.js
tests/functional/testPreferences/testDefaultPhishingEnabled.js
tests/functional/testPreferences/testDefaultSecurityPrefs.js
tests/functional/testPreferences/testSwitchPanes.js
tests/functional/testSearch/testAddMozSearchProvider.js
tests/functional/testSearch/testRemoveSearchEngine.js
tests/functional/testSearch/testReorderSearchEngines.js
tests/functional/testSecurity/testSafeBrowsingNotificationBar.js
tests/functional/testSecurity/testSafeBrowsingWarningPages.js
tests/functional/testSecurity/testSSLDisabledErrorPage.js
tests/functional/testSecurity/testUnknownIssuer.js
tests/functional/testTabbedBrowsing/testBackgroundTabScrolling.js
tests/functional/testTabbedBrowsing/testCloseTab.js
tests/functional/testTabbedBrowsing/testOpenInForeground.js
lib/downloads.js
lib/tests/testDownloads.js
lib/tests/testSearch.js
lib/utils.js
lib/widgets.js

There may be some that are ALLCAPS, but aren't seprarated by an underscore where they should be ALL_CAPS. But I couldn't think of a clever way to find those.

If you'd like me to fix these known ones, feel free to assign me and I'll be happy to do it.
Absolutely! Would be great to see the same style for constant names. Thanks Jonathan! I will assign it to you.
Assignee: nobody → tojonmz
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Whiteboard: [mentor=andreea][lang=js][good first bug]
(Assignee)

Comment 2

6 years ago
Sure, no problems. I will look at this after or in-between work on 2-3 other bugs I'm currently looking at.
(Assignee)

Comment 3

6 years ago
Created attachment 723208 [details] [diff] [review]
clean up non-cap constants (default)

Patch "clean up non-cap constants (default)" for the default branch. Twenty three files affected.

There were instances of constants gTimeout,gDelay which were not used in some tests. They have been capitalized, but potential removal from the tests altogether can be addressed in bug 848200.

There were some cases where a path was being assembled again in controller.open(). Those were refactored into the standard LOCAL_TEST_LOCATION, LOCAL_TEST_PAGE structure near the top of the test, similar to the work done in bug 848361. Related to this - it might be an idea to review and land that patch for that bug first, before landing this one. A few of the files intersect both patches and this patch adds a few new lines, in places. But I could tweak that other patch if needed if this is landed first.

Particular attention in the review should be paid to download.js in this bug's review. 'downloadState' appeared to be getting used syntactically as both a constant and a property. With this change, the constant is updated to ALL_CAPS, and the property remains unchanged. As you can see below, the key pair constant got used for a property of the same name, for the window object when the constructor is called. It is also used again at the bottom of the module to provide the same array values for the test lib/tests/testDownloads.js, when it is run and the downloadState is recorded. I made a point to confirm this by intentionally breaking the module, by altering the property value and running the test.

downloads.js (post change)

(line30)
const DOWNLOAD_STATE = {
  notStarted      : -1,
  downloading     : 0,
  finished        : 1,
  ......etc

/**
 * Constructor
 */
function downloadManager() {
  this._controller = null;
  this.downloadState = DOWNLOAD_STATE;
}

(line 462)
// Export of variables
exports.downloadState = DOWNLOAD_STATE;


Tested with Default Nightly 22.0a1 20130309030841. Tests pass where expected. lib/tests/testDownloads contains one known fail, and tests/functionality/testSecurity/testUnknownIssuer in its entirety, is a known skip.

Let me know if there's anything you'd like adjusted.
Attachment #723208 - Flags: review?(hskupin)
Attachment #723208 - Flags: review?(hskupin) → review?(andreea.matei)
Comment on attachment 723208 [details] [diff] [review]
clean up non-cap constants (default)

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

Awesome work Jonathan! So glad to see such consistency in our tests, thanks!

There were some cases where we had timeout and delay consts that were not being used at all in the test so we can safely remove them.
After removing those we'll be good to land!

::: lib/downloads.js
@@ +453,1 @@
>                                 .QueryInterface(Ci.nsIFileURL);

Can you please move this to the right so the dots are aligned?

::: lib/tests/testSearch.js
@@ +8,5 @@
>  var search = require("../search");
>  var utils = require("../utils");
>  
> +const G_DELAY = 0;
> +const G_TIMEOUT = 5000;

I don't see these being used in the code, don't know why were here. So we can remove G_TIMEOUT, use G_DELAY = 500 and replace 500 in the sleep calls from the test.

::: lib/utils.js
@@ +15,5 @@
>  // Include required modules
>  var { assert } = require("assertions");
>  var prefs = require("prefs");
>  
> +const G_TIMEOUT = 5000;

It's not being used here either.

::: lib/widgets.js
@@ +9,5 @@
>  
>  var EventUtils = {};
>  Cu.import('resource://mozmill/stdlib/EventUtils.js', EventUtils);
>  
> +const G_TIMEOUT = 5000;

Same here.

::: tests/functional/testAwesomeBar/testLocationBarSearches.js
@@ +6,5 @@
>  var { assert } = require("../../../lib/assertions");
>  var prefs = require("../../../lib/prefs");
>  var toolbars = require("../../../lib/toolbars");
>  
> +const G_TIMEOUT = 5000;

We can remove this now.

::: tests/functional/testInstallation/testBreakpadInstalled.js
@@ +12,4 @@
>                     "darwin" : "crashreporter.app",
>                     "win32"  : "crashreporter.exe",
>                     "linux"  : "crashreporter"
>                    };

This needs to be aligned with the opening one please and the elements moved a space to the right.

::: tests/functional/testPopups/testPopupsAllowed.js
@@ +13,5 @@
>  
>  const PREF_POPUP_BLOCK = "dom.disable_open_during_load";
>  
> +const G_DELAY = 0;
> +const G_TIMEOUT = 5000;

These two are not being used here, we can remove them.

::: tests/functional/testPopups/testPopupsBlocked.js
@@ +13,4 @@
>  
>  const PREF_POPUP_BLOCK = "dom.disable_open_during_load";
>  
> +const G_DELAY = 0;

We can remove Delay const is not used in the test.

::: tests/functional/testPreferences/testDefaultPhishingEnabled.js
@@ +5,5 @@
>  // Include necessary modules
>  var { expect } = require("../../../lib/assertions");
>  var prefs = require("../../../lib/prefs");
>  
> +const G_DELAY = 0;

Same here.

::: tests/functional/testPreferences/testDefaultSecurityPrefs.js
@@ +5,5 @@
>  // Include necessary modules
>  var { expect } = require("../../../lib/assertions");
>  var prefs = require("../../../lib/prefs");
>  
> +const G_DELAY = 0;

And here.

::: tests/functional/testSearch/testAddMozSearchProvider.js
@@ +12,5 @@
>  
>  const TIMEOUT_INSTALL_DIALOG = 30000;
>  
> +const SEARCH_ENGINE = {name: "mozqa.com",
> +                      url : LOCAL_TEST_FOLDER + "search/mozsearch.html"};

Please move this one space to the right so it's aligned with name.

::: tests/functional/testSearch/testRemoveSearchEngine.js
@@ +6,5 @@
>  var { assert, expect } = require("../../../lib/assertions");
>  var search = require("../../../lib/search");
>  
> +const G_DELAY = 0;
> +const G_TIMEOUT = 5000;

We can remove them as they are not being used

::: tests/functional/testSearch/testReorderSearchEngines.js
@@ +6,5 @@
>  var { expect } = require("../../../lib/assertions");
>  var search = require("../../../lib/search");
>  
> +const G_DELAY   = 0;
> +const G_TIMEOUT = 5000;

G_TIMEOUT is not used.

::: tests/functional/testSecurity/testSSLDisabledErrorPage.js
@@ +10,5 @@
>  const PREF_KEEP_ALIVE = "network.http.keep-alive";
>  const PREF_SSL_3 = "security.enable_ssl3";
>  const PREF_TLS = "security.enable_tls";
>  
> +const G_DELAY = 0;

G_DELAY is not being used

::: tests/functional/testSecurity/testSafeBrowsingNotificationBar.js
@@ +6,5 @@
>  var { assert, expect } = require("../../../lib/assertions");
>  var tabs = require("../../../lib/tabs");
>  var utils = require("../../../lib/utils");
>  
> +const G_DELAY = 0;

This can be removed.

::: tests/functional/testSecurity/testSafeBrowsingWarningPages.js
@@ +8,5 @@
>  var tabs = require("../../../lib/tabs");
>  var utils = require("../../../lib/utils");
>  
>  
> +const G_DELAY = 0;

Here as well.

::: tests/functional/testSecurity/testUnknownIssuer.js
@@ +4,5 @@
>  
>  // Include necessary modules
>  var { assert, expect } = require("../../../lib/assertions");
>  
> +const G_DELAY = 0;

Same here.

::: tests/functional/testTabbedBrowsing/testCloseTab.js
@@ +6,5 @@
>  var { assert } = require("../../../lib/assertions");
>  var tabs = require("../../../lib/tabs");
>  
> +const G_DELAY = 0;
> +const G_TIMEOUT = 5000;

We can remove these 2 here.

::: tests/functional/testTabbedBrowsing/testOpenInForeground.js
@@ +13,5 @@
>  
>  const PREF_TAB_LOAD_IN_BACKGROUND = "browser.tabs.loadInBackground";
>  
> +const G_DELAY = 0;
> +const G_TIMEOUT = 5000;

And here as well.
Attachment #723208 - Flags: review?(andreea.matei) → review-
I just want to add that we can even remove those timeouts which are 5s and still in use for waitForElement. 5s is the default used internally so we can strip those off too.
Whiteboard: [mentor=andreea][lang=js][good first bug] → [mentor=andreeamatei][lang=js][good first bug]
Thanks Henrik. 
For waitThenClick() as well, I've updated the mdn page with this timeout:
https://developer.mozilla.org/en-US/docs/Mozmill/Mozmill_Element_Object
(Assignee)

Comment 7

6 years ago
(In reply to Andreea Matei [:AndreeaMatei] from comment #4)
> Comment on attachment 723208 [details] [diff] [review]
> clean up non-cap constants (default)
> 
> Review of attachment 723208 [details] [diff] [review]:
> -----------------------------------------------------------------

Sure, no problems Andreea. I wasn't sure how aggressive we wanted to be with purging the unused gdelay,gTimeout during the same work, I had originally thought we might do that as part of the purge of unused 5000ms timeouts in bug 848200. Sort of capitalize everything first, then purge. But no problems, I will do that with this change. I will make all the other tweaks as described.

If it helps you speeding up later reviews, you can always say 'remove unused x,y or z' in this type of scenario if it saves you time itemizing each, I will find them all no problems. Thanks for the other subtlety ones though, I will address those as described.

I'm inclined to leave the used 5000ms timeouts (default waitForElement), and address their purge in bug 848200? If that's ok? But if you guys would prefer I do it here in this bug I will, no problems.
We might want to do it the other way around. I don't see a reason why to modify constants which will be removed right after. So I would say get bug 848200 fixed first. :)
(Assignee)

Comment 9

6 years ago
Cool, no probs :) I've already started on the updates to this patch including purging unused gDelay,gTimeout, so I'll upload a new patch for this bug shortly, and then do the other bug also. Whichever patch gets landed first, I'll do a new pull, and do any resolves if needed and freshen up the 'other' patch for you. 

That other bug initial intent, was to purge unused occurrences of 'regular' timeout, rather than gTimeout. Though there could be some other variants I've yet to encounter, and may find during that work :)
(Assignee)

Comment 10

6 years ago
Created attachment 724048 [details] [diff] [review]
clean up non-cap constants (default) rev 1.1

Patch "clean up non-cap constants (default) rev1.1" for the default branch. Twenty three files affected.

Instances of former constants gTimeout,gDelay were removed where unused, as per Comment #4 review. testPreferences/testDefaultSecurityPrefs actually references G_DELAY in the test, though its initial value is set to 0, so it was left in as part of the fix for now. Whitespace adjusted as specified. I should have caught those. For some reason I mapped an equivalent number of characters in my head, but in some cases they were altering the line length.

Tested with Default Nightly 22.0a1 20130312031046. All tests pass where expected.

This patch supersedes the original for default, so marking that one as obsolete.
Attachment #723208 - Attachment is obsolete: true
Attachment #724048 - Flags: review?(andreea.matei)
Comment on attachment 724048 [details] [diff] [review]
clean up non-cap constants (default) rev 1.1

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

Great, it's fine by me since you already started to work on this. Looks good. 
Looking forward for the fix in the other bug as well :)

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/28b60116c6fb
Attachment #724048 - Flags: review?(andreea.matei) → review+
status-firefox22: --- → fixed
Comment on attachment 724048 [details] [diff] [review]
clean up non-cap constants (default) rev 1.1

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

::: lib/downloads.js
@@ +17,5 @@
>  var domUtils = require("dom-utils");
>  var prefs = require("prefs");
>  var utils = require("utils");
>  
> +const G_TIMEOUT = 5000;

While this is fine the 'G_' prefix would not have been necessary here. We started to use the hungarian notation at the beginning but then decided otherwise. Can we do a follow-up for those tests which are affected before we backport?
(Assignee)

Comment 13

6 years ago
Absolutely. I will replace all occurrences of gTimeout(or G_TIMEOUT) with TIMEOUT in default for this bug, with another patch. Then when backporting I gather I will combine both patches, into a single patch for aurora, beta, release, esr17.
(Assignee)

Comment 14

6 years ago
Created attachment 724439 [details] [diff] [review]
clean up non-cap constants (default) supplementary - rev 2.0

Patch "clean up non-cap constants (default) supplementary - rev 2.0" for the default branch. Ten files modified. 

Converted all occurrences of G_TIMEOUT to TIMEOUT. Once all the many changes for various bugs have settled down across all repos (particularly bug 848649), I will later remove instances of TIMEOUT like these, where the wait is the default 5000ms, in bug 848200.

Tested with Default Nightly 22.0a1 20130313031041. Tests pass where expected.

Tests testSecurity/testUnknownIssuer is a known skip, lib/tests/testDownloads is a known partial fail (2pass,1fail).

This patch is supplementary to the earlier patch rev1.1 for default, not a replacement, it should be landable as it is resolved against a new pull.

For backports to aurora,beta,release,esr17 if applicable, a unified patch will be generated.
Attachment #724439 - Flags: review?(andreea.matei)
Comment on attachment 724439 [details] [diff] [review]
clean up non-cap constants (default) supplementary - rev 2.0

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

While the work for TIMEOUT is complete, I would like to see the same for G_DELAY before landing this.

::: lib/tests/testDownloads.js
@@ +4,5 @@
>  
>  // Include required modules
>  var downloads = require("../downloads");
>  
>  const G_DELAY = 0;

Would you mind changing this constant as well with DELAY?
Applies to all occurrences in the repository.
Attachment #724439 - Flags: review?(andreea.matei) → review-
(Assignee)

Comment 16

6 years ago
Sure, no problems. I will convert all occurrences of G_DELAY to DELAY. Will have a patch shortly. 

I'll want to do another resolve though before uploading, after a supplementary patch for bug 848361 is landed first. Just to be sure this patch will land cleanly on top. Some files intersect both bugs.
(Assignee)

Comment 17

6 years ago
Created attachment 724955 [details] [diff] [review]
clean up non-cap constants (default) supplementary - rev 2.1

Patch "clean up non-cap constants (default) supplementary - rev 2.1" for the default branch. Thirteen files modified. 

Converted all occurrences of G_TIMEOUT,G_DELAY, and TIMEOUT,DELAY. 

Removed G_DELAY completely from lib/tests/testDownloads, as it was unused.

Once all the many changes for various bugs have settled down across all repos (particularly bug 848649), I will later remove instances of TIMEOUT like these, where the wait is the default 5000ms, in bug 848200.

Tested with Default Nightly 22.0a1 20130314030914. Tests pass where expected.

Tests testSecurity/testUnknownIssuer is a known skip, lib/tests/testDownloads is a known partial fail (2pass,1fail).

This patch is supplementary to the earlier patch rev1.1 for default, not a replacement. Perhaps wait to try landing this patch, after bug 848361's patch has been landed on default, some files may intersect. If there's any issues I will resolve either of them as needed.

For backports to aurora,beta,release,esr17 if applicable, a unified patch will be generated.
Attachment #724439 - Attachment is obsolete: true
Attachment #724955 - Flags: review?(andreea.matei)
(Assignee)

Comment 18

6 years ago
(In reply to Jonathan French (:jfrench) from comment #17)
> 
> Converted all occurrences of G_TIMEOUT,G_DELAY, and TIMEOUT,DELAY. 

Meant to write "to TIMEOUT,DELAY", here.
Comment on attachment 724955 [details] [diff] [review]
clean up non-cap constants (default) supplementary - rev 2.1

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

Awesome, we're good for default now.
Just that the patch is no longer applying, probably because of landing from bug 848361. 
Would you mind saving the patch with .patch extension?

applying patchBug848653_Default_rev2_1.txt
patching file tests/functional/testSecurity/testSSLDisabledErrorPage.js
Hunk #1 FAILED at 5
Hunk #2 FAILED at 47
2 out of 2 hunks FAILED -- saving rejects to file tests/functional/testSecurity/testSSLDisabledErrorPage.js.rej
patching file tests/functional/testSecurity/testUnknownIssuer.js
Hunk #1 FAILED at 0
1 out of 1 hunks FAILED -- saving rejects to file tests/functional/testSecurity/testUnknownIssuer.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh patchBug848653_Default_rev2_1.txt
Attachment #724955 - Flags: review?(andreea.matei) → review-
(Assignee)

Comment 20

6 years ago
Sure, makes sense with that other new patch just landed. Will also use .patch no problems.
(Assignee)

Comment 21

6 years ago
Created attachment 725398 [details] [diff] [review]
clean up non-cap constants (default) supplementary - rev 2.2

Patch "clean up non-cap constants (default) supplementary - rev 2.2" for the default branch. Thirteen files modified.

Converted all occurrences of G_TIMEOUT,G_DELAY, to TIMEOUT,DELAY.

Additional resolves were anticipated, now completed. 

Tested with Default Nightly 22.0a1 20130315030943. Tests pass where expected.

This patch is supplementary to the earlier patch rev1.1 for default, not a replacement. For backports to aurora,beta,release,esr17 if applicable, a unified patch will be generated.

This patch supersedes the earlier rev2.1 for default, so marking that one as obsolete.
Attachment #724955 - Attachment is obsolete: true
Attachment #725398 - Flags: review?(andreea.matei)
Comment on attachment 725398 [details] [diff] [review]
clean up non-cap constants (default) supplementary - rev 2.2

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

Great! Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/36eab0ecd471 (default)

Looking forward to backporting :)

Thanks!
Attachment #725398 - Flags: review?(andreea.matei) → review+
(Assignee)

Comment 23

6 years ago
Cool, will start that early this week.
(Assignee)

Comment 24

6 years ago
Created attachment 726369 [details] [diff] [review]
clean up non-cap constants (aurora) rev 1.0

Patch "clean up non-cap constants (aurora) - rev 1.0" for the Aurora branch. Twenty five files modified.

This patch represents the union of the original two patches (rev1.1,supplementary2.2) landed earlier on default. However two other files, not in the initial 'scope' for capitalization fixes, were also lurking in the repo with unwanted gTimeout,gDelay. They have been corrected and included with this Aurora patch. 

Those two files which are listed below, will require an additional supplementary patch for Default.

lib\software-update.js
tests\functional\testSecurity\testIdentityPopupOpenClose.js

Tested with Aurora 21.0a2 20130317042012. All tests pass where expected. lib/tests/testDownloads is a known partial-fail, testSecurity/testUnknownIssuer is a known skip.

I will provide that additional supplementary patch (supplementary rev3.0) for Default, shortly.
Attachment #726369 - Flags: review?(andreea.matei)
(Assignee)

Updated

6 years ago
Summary: Clean up non capitalized constants → Clean up non capitalized constants, purge gTimeout,gDelay
(Assignee)

Comment 25

6 years ago
Created attachment 726384 [details] [diff] [review]
clean up non-cap constants (default) supplementary - rev3.0

Patch "clean up non-cap constants (default) supplementary - rev 3.0" for the Default branch. Two files modified.

Two files not in the initial 'scope' for capitalization fixes, were lurking in the repo with unwanted gTimeout,gDelay. They have been corrected and included in this supplementary patch for Default only.

lib\software-update.js
tests\functional\testSecurity\testIdentityPopupOpenClose.js

Tested with Default 22.0a1 20130315030943. Test passes as expected.
Attachment #726384 - Flags: review?(andreea.matei)
Comment on attachment 726384 [details] [diff] [review]
clean up non-cap constants (default) supplementary - rev3.0

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

Pushed to default:
http://hg.mozilla.org/qa/mozmill-tests/rev/3ebf88afaff3
Attachment #726384 - Flags: review?(andreea.matei) → review+
Comment on attachment 726369 [details] [diff] [review]
clean up non-cap constants (aurora) rev 1.0

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

Great to have a full patch :)
http://hg.mozilla.org/qa/mozmill-tests/rev/93be043ec5c7 (aurora)
Attachment #726369 - Flags: review?(andreea.matei) → review+
status-firefox21: --- → fixed
(Assignee)

Comment 28

6 years ago
Created attachment 726678 [details] [diff] [review]
clean up non-cap constants (beta) rev 1.0

Patch "clean up non-cap constants (beta) - rev 1.0" for the Beta branch. Twenty five files modified.

This patch represents the union of the original patches landed earlier on default, and then unified in Aurora.

There were additional resolves required for Beta. No additional occurrences of gDelay,gTimeout or non-capitalized constants, unique to Beta, existed.

Tested with Beta 20.0 20130313170052. Tests pass where expected.
Attachment #726678 - Flags: review?(andreea.matei)
(Assignee)

Comment 29

6 years ago
Created attachment 726876 [details] [diff] [review]
clean up non-cap constants (release) rev 1.0

Patch "clean up non-cap constants (release) - rev 1.0" for the Release branch. Twenty nine files modified.

This patch represents the original patches landed earlier on default, then unified in Aurora,Beta.

There were additional resolves required for Release. Also, four files unique to Release required updates. They are:

tests/crash/testRemoveCookieAfterPrivateBrowsing.js
tests/functional/testPrivateBrowsing/testDisabledElements.js
tests/functional/testPrivateBrowsing/testGeolocation.js
lib/private-browsing.js

Tested with Release 19.0.2 20130307023931. Tests pass where expected.
Attachment #726876 - Flags: review?(andreea.matei)
(Assignee)

Comment 30

6 years ago
Created attachment 726911 [details] [diff] [review]
clean up non-cap constants (esr17) rev 1.0

Patch "clean up non-cap constants (esr17) - rev 1.0" for the Esr17 branch. Thirty files modified.

This patch represents the backport of the Release patch(which itself is a rollup of Default,Beta,Aurora patches), to Esr17.

No resolves were required, however one file unique to Esr17 required update, to purge gDelay,gTimeout. It is:

testSecurity/testEncryptedPageWarning

Tested with Esr17 Nightly 17.0.4esrpre 20130317034503. Tests pass where expected.
Attachment #726911 - Flags: review?(andreea.matei)
Comment on attachment 726911 [details] [diff] [review]
clean up non-cap constants (esr17) rev 1.0

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

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/de3ca37ad9ce (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/f3241487f946 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/ecdf2a62f0fd (esr17)

Thanks Jonathan, great work! We're done here :)
Attachment #726911 - Flags: review?(andreea.matei) → review+
Attachment #726678 - Flags: review?(andreea.matei) → review+
Attachment #726876 - Flags: review?(andreea.matei) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
status-firefox19: --- → fixed
status-firefox20: --- → fixed
status-firefox-esr17: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.