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.

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

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

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