Page MenuHomePhabricator

ResourceLoaderFileModule should have a debug mode for styles
Closed, DuplicatePublic

Description

Author: mdale

Description:
In debug mode it would be ideal if the css files could be directly referenced rather than inline included.

At least the css files should not be compressed in debug mode.


Version: unspecified
Severity: enhancement

Details

Reference
bz27025
ReferenceSource BranchDest BranchAuthorTitle
repos/releng/dev-images!1brennen/update-docsmainbrennenclean up references to gerrit
Customize query in GitLab

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:17 PM
bzimport set Reference to bz27025.
bzimport added a subscriber: Unknown Object (MLST).

mdale wrote:

Note this would also help situations where your dealing with dynamic paths for the script include. ie in remote embedding or when you have a stand lone test file outside the root directory of the resource loader.

When inserting transformed files into the current page, the resource loader does not take into consideration the current path of the page its acting on, ie a runtime configuration setting relative path. With the current setup you have to set $wgScriptPath to an absolute url.

Would be nice" to support non-transformed css output so that debugging css error is not as tricky.

I'd say this is a bug rather than a feature request if you ask me. Debug mode means not combined, but linked directly, that goes for CSS as well.

mdale wrote:

I have to agree with Krinkle, its very difficult to debug css once you have a half dozen files with cascading rules all in same page without any reference to what files they came from.

Since ResourceLoader should not make things harder but instead make things easier adding this as a blocker to the general 'Implement ResourceLoader' as the current situation is sort of a regression.

The only place this breaks down is RTL - because we support that on the fly. Otherwise CSS could be included directly just as JS is with no issues.

mdale wrote:

Even for RTL you should load the style sheet in another "&only=styles" request, otherwise you have every style sheet in the same dynamic include page append ... making it difficult to trace which rules come from where

Removing odd dependency on bug 26939

Sensible thing for RTL could be to load them as individual <link rel="stylesheet">s but still run them through load.php for transformation.

What's the status on this in regards to bug 30804, which claims that RTL is now broken in debug mode?

  • Bug 30804 has been marked as a duplicate of this bug. ***

I don't really mind whether we do or do not load CSS in debug mode though load.php or from disk directly. What I do care about, is these two points:

  1. Not minified,

so that advanced developer tools such as those that come with Google Chrome (not the default web inspector, but the more elaborate tools such as in-file editing of referenced resoruces)

  1. Should eventually end up in a place other than the main document's DOM.

Reason being that many developer tool mark those styles as "in-document" style overrides (which, in a way, they then are).

Note that these two points don't require loading them from disk. Using only=styles in a separate request in <link> works too. I don't have a preference for which one.

Krinkle -- since you marked 30804 as a dupe of this, I assume you know: can you clarify what the present status of this is and describe how the RTL CSS got broken?

mdale wrote:

Krinkle, I would agree. All that maters is that you a way to find out what came from where.. if you have to read the url to get the style name that’s fine as you can relatively easily find where the rule came from.

(In reply to comment #12)

  1. Should eventually end up in a place other than the main document's DOM.

Reason being that many developer tool mark those styles as "in-document" style
overrides (which, in a away, then are).

If I recall correctly, there are browsers in which adding <link> tags dynamically doesn't work but adding <style> tags does, and that's why we chose the latter back when we developed RL. However I think it was just one browser that was old and shitty (but still supported at the time), so it may be defensible to break that browser in debug mode. I just wish I remembered which one it was.

darklama wrote:

If styles aren't minified in debug mode, wouldn't prepending a comment
as each stylesheet is combined together do the trick?

/ /skins/common/shared.css /

abbr, acronym, .explain { }
...

/ /resources/mediawiki.special/mediawiki.special.preferences.css /

#mw-emailaddress-validity { }
...

/ MediaWiki:Common.css /

...

mdale wrote:

Prepending a comment to each style is not as ideal as separate requests because your looking at these styles via a debug tool like firebug, which has an interface organized around css coming from particular files.

In firebug you can click through to the style location in-page then scroll up, but a separate request is much more ideal so that you can get a quick summary of what rules come from where and how they are cascading to build the given in-page representation.

Adding the comment is not a useful fix. Implementing something like that would require lots of changes that simply aren't needed. We can just fix this bug the "right way".