Page MenuHomePhabricator

Add support for behavior: url(#default#behaviorName)
Closed, ResolvedPublic

Description

The Mapbox/leaflet upgrade (T147347) introduces a CSS property in the default mapbox style that currently makes ResourceLoader tests fail. See T147347#3111300

Patch: https://gerrit.wikimedia.org/r/#/c/317546/

The property that makes it fail is in lib/mapbox/style.css :

behavior:url(#default#VML);

It's kind of a weird property but it's valid... http://stackoverflow.com/questions/26339276/what-is-behavior-url-property-in-css

  • Can we add this special case to CSSMin::getLocalFileReferences ?
  • Otherwise, can we add a special comment like /* @ignoreurl */?

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 14 2017, 12:14 AM
Krinkle removed JGirault as the assignee of this task.Apr 22 2017, 1:19 AM

(Un-assigning - I assume this was the unintended default from creating a subtask based on the parent).


This bug is similar to T151972: CSSMin should not convert urls for filters. However in this case there is one notable exception. The url values in filter always refer to a relative url, which means hash values will refer to an element on the page. This is intentionally not supported by ResourceLoader, as explained in T151972#2897445..

However, in the case of behaviour the value can be one of three things, as shown by http://stackoverflow.com/a/26339480/319266:

eirenaios at Stack Overflow:
  • url(sLocation): [..] where sLocation is an absolute or relative URL.
  • url(#objID): [..] where objID is the ID [of] an object tag [on the page].
  • url(#default#behaviorName): [..] The application's default behavior, identified by its behaviorName.

This last one does not refer to an element on the page, rather it refers to special reserved words that just happen to look like a hash fragment.

Excellent
Thanks @Krinkle for the details.

I listed two possible solutions in the ticket. Any advice?
I'd like to work on the patch, because this is blocking mapbox.js upgrade.

Change 350746 had a related patch set uploaded (by JGirault; owner: JGirault):
[mediawiki/core@master] Skip #default#behaviorName when detecting local files

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

Krinkle assigned this task to JGirault.May 26 2017, 2:20 PM

Change 350746 had a related patch set uploaded (by JGirault; owner: JGirault):
[mediawiki/core@master] Skip #default#behaviorName when detecting local files
https://gerrit.wikimedia.org/r/350746

Tests are currently failing.

TheDJ added a subscriber: TheDJ.Jul 22 2017, 11:55 PM

Revived and fixed testcase.

Krinkle closed this task as Resolved.Jul 24 2017, 7:37 PM
Krinkle reassigned this task from JGirault to TheDJ.
Krinkle removed a project: Patch-For-Review.

@TheDJ Thanks!

Change 350746 merged by jenkins-bot:
[mediawiki/core@master] CSSMin: Skip #default#behaviorName when detecting local files

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

Change 437474 had a related patch set uploaded (by Hashar; owner: JGirault):
[mediawiki/core@REL1_27] CSSMin: Skip #default#behaviorName when detecting local files

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

Change 437474 abandoned by Umherirrender:
CSSMin: Skip #default#behaviorName when detecting local files

Reason:
REL1.27 is out of support, new LTS is REL1.31

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