Page MenuHomePhabricator

EventLogging dependency isn't so optional
Closed, ResolvedPublic

Description

Looking through the code briefly, it looks like Popups conditionally loads the schema module that EL uses, but loads the ext.popups.eventlogging.js unconditionally and uses the functions defined there without any conditions as well.

I'd suggest putting a check for the EventLogging library in front of the call to mw.eventLog.logEvent() in that file...at the very least. Hopefully avoid loading the file at all if EL is turned off.

Details

Reference
bz68341

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:36 AM
bzimport added a project: Page-Previews.
bzimport set Reference to bz68341.
bzimport added a subscriber: Unknown Object (MLST).
Prtksxna triaged this task as High priority.Dec 2 2014, 4:13 PM
Prtksxna updated the task description. (Show Details)
Prtksxna set Security to None.

Change 183765 had a related patch set uploaded (by Prtksxna):
Add a check for the EventLogging library before the logEvent call

https://gerrit.wikimedia.org/r/183765

Patch-For-Review

Prtksxna claimed this task.Jan 9 2015, 2:18 AM
Jaredzimmerman-WMF moved this task from Next Up to For Review on the Page-Previews board.

Change 183765 merged by jenkins-bot:
Add a check for the EventLogging library before the logEvent call

https://gerrit.wikimedia.org/r/183765

Prtksxna closed this task as Resolved.Feb 27 2015, 12:27 AM
Prtksxna moved this task from For Review to Done on the Page-Previews board.Mar 12 2015, 10:25 AM