Closed Bug 1490627 Opened Last year Closed Last year

browser-captivePortal.js leaks cps variable into the browser window's global scope

Categories

(Firefox :: General, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Firefox 64
Tracking Status
firefox64 --- fixed

People

(Reporter: dao, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
No description provided.
Attachment #9008363 - Flags: review?(nhnt11)
Comment on attachment 9008363 [details] [diff] [review]
patch

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

Nice catch, thanks.

::: browser/base/content/browser-captivePortal.js
@@ -1,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> -XPCOMUtils.defineLazyServiceGetter(this, "cps",

What do you think about keeping this lazy service getter but s/this/CaptivePortalWatcher/ so that the property is defined on the CaptivePortalWatcher object?
Attachment #9008363 - Flags: review?(nhnt11) → review+
(In reply to Nihanth Subramanya [:nhnt11] from comment #1)
> What do you think about keeping this lazy service getter but
> s/this/CaptivePortalWatcher/ so that the property is defined on the
> CaptivePortalWatcher object?

This is accessed in init so there's no point in keeping this lazy. It would just add overhead.
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c2e6f132f46
browser-captivePortal.js leaks cps variable into the browser window's global scope. r=nhnt11
https://hg.mozilla.org/mozilla-central/rev/6c2e6f132f46 - landed 22 hours ago
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Blocks: 1509029
You need to log in before you can comment on or make changes to this bug.