Page MenuHomePhabricator

Popups should break without TextExtracts and PageImages installed
Closed, ResolvedPublic


Just wasted 10 minutes of my life debugging why Popups wasn't working.
Discovered TextExtracts was not installed.

Extension page says "This extension has a hard dependency on Extension:TextExtracts and Extension:PageImages." but the extension doesn't enforce these. If they are hard dependencies please enforce this in the ExtensionSetup hook.

Event Timeline

Jdlrobson created this task.Feb 1 2016, 9:59 PM
Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: Page-Previews.
Jdlrobson added a subscriber: Jdlrobson.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptFeb 1 2016, 9:59 PM
jhobs triaged this task as Low priority.Feb 2 2016, 6:10 PM
jhobs set Security to None.
Jdlrobson raised the priority of this task from Low to Normal.Apr 20 2016, 5:49 PM

I just wasted 15 minutes debugging something caused by this issue again.

The Popups beta feature is added to Special:Preferences provided that PopupsBetaFeature is true.

The ResourceLoader module however only adds the module if this is true, the beta feature is enabled, BetaFeatures, TextExtracts and PageImages extensions are installed.

We should create a helper method isPopupsEnabled that encapsulates, simplifies and standardises all this logic for the sanity of everyone and to avoid more time wasting.

If TextExtracts and PageImages are not installed the beta feature should not show up in Special:Preferences

Jdlrobson lowered the priority of this task from Normal to Low.Jul 28 2016, 8:46 PM
Jdlrobson raised the priority of this task from Low to Normal.
Jdlrobson added a project: Technical-Debt.
Jdlrobson moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
phuedx added a subscriber: phuedx.Dec 21 2016, 9:30 AM

This should be trivial once rEPOP4958e40c5ce5: Hygiene: Improve code quality by reaching 100% coverage is merged as it introduces PopupsContext#areDependenciesMet.

Jdlrobson closed this task as Resolved.Feb 14 2017, 9:38 PM
Jdlrobson claimed this task.

I see this in the error log:

[popups] Popups requires the PageImages and TextExtracts extensions. If Beta mode is on it requires also BetaFeatures extension

So I guess this is better than nothing :)