Opened 12 years ago

Closed 4 years ago

#5402 closed enhancement (patchwelcome)

dojo/css! plugin

Reported by: James Burke Owned by: Kris Zyp
Priority: low Milestone: 1.13
Component: Loader Version: 1.0
Keywords: reviewed Cc: Rawld Gill
Blocked By: Blocking:

Description (last modified by bill)

Consider including some form of a dojo.requireCss for 1.1. There is a good summary on dojo-contributors by Bill of the options. This is just a placeholder ticket for now as a reminder, and as a final design is worked out, post it here.

See

Attachments (4)

requireCss.js (4.2 KB) - added by James Burke 11 years ago.
First shot at requireCss, but has issue with cross domain css in Firefox: we cannot determine when it gets loaded.
requireCss.html (954 bytes) - added by James Burke 11 years ago.
Test file. Place as a sibling to the dojo dir.
phlebotomy.png (147.5 KB) - added by codeseo 6 years ago.
http://phlebotomyguide.org
main.inc.php (1.6 KB) - added by Slavon 6 years ago.
http://remedyby.tumblr.com/

Download all attachments as: .zip

Change History (29)

comment:1 Changed 12 years ago by James Burke

Attached a first shot at this. The big blocker right now is Firefox and css files that are loaded from another domain. Normally the linkNode.sheet.cssRules property could tell us if the file was loaded (or at least parsed, not sure about applied). Unfortunately, Firefox complains with a security error if cssRules is accessed when the css file is loaded from another domain. Safari 3 (on mac, haven't tested windows) seems to wait to define linkNode.sheet until the file has been loaded, so that works out in the xd case.

So, just xd css in Firefox is the holdup. Unfortunately I do not see an easy way around the issue yet. I tried in FF 3 beta 2, and same issue as FF 2.

The only thing I can think of is accepting a dojo.query string and a style property that should be set if the css file is loaded, but this is pretty weak: first you have to be sure the queried node will be in the document and match a style in the stylesheet, and since we would be inspecting a node.style.property, *no other stylesheet* should be setting that property. So yeah, very lame.

comment:2 Changed 12 years ago by bill

Description: modified (diff)

comment:3 Changed 12 years ago by bill

Hi James. Sorry, no bright ideas about FF. We could require every CSS file to have some CSS class name defined, related to the name of the file itself, but that's a bit unpleasant.

Don't forget also that CSS files can include other CSS files (and we do this in dijit -- tundra.css includes dijit.css). Not sure how to handle that.

comment:4 Changed 11 years ago by James Burke

Milestone: 1.11.2

comment:5 Changed 11 years ago by bill

FYI, this would solve #4058 and #5824 in a non-hackish way. (But we have a hack ways to fix them already, so no rush.)

Changed 11 years ago by James Burke

Attachment: requireCss.js added

First shot at requireCss, but has issue with cross domain css in Firefox: we cannot determine when it gets loaded.

Changed 11 years ago by James Burke

Attachment: requireCss.html added

Test file. Place as a sibling to the dojo dir.

comment:6 Changed 11 years ago by bill

Also need to confirm that (for example) doing a dojo.requireCss("tundra.css"); wouldn't be marked as completed until tundra's imports (like dijit.css and TabContainer?.css) had finished downloading.

comment:7 Changed 11 years ago by James Burke

The issue that is blocking a checkin of this code is outlined in this mozilla bug. If that bug gets traction, we might have something worth trying:

https://bugzilla.mozilla.org/show_bug.cgi?id=185236

comment:8 Changed 11 years ago by James Burke

Description: modified (diff)
Milestone: 1.21.3

Moving to 1.3. Need to see movement on the mozilla bug mentioned above. See if anything happens in the 1.3 timeframe.

comment:9 Changed 11 years ago by dylan

Given that this is blocked on a 6-year-old Mozilla bug, what should we do? Can we focus instead on a build system approach to roll-up all import statements? Can we add cssTemplatePath back as a mixin for Dijit (like Templated, and again make it an optional thinger)?

comment:10 Changed 11 years ago by James Burke

Milestone: 1.3future

The build system already inlines @import calls inside CSS files, are you suggesting that the build system could process requireCss calls, and inline the CSS as a string in the JS file, then add the file's content to a newly created style node in the page?

I guess that might be an option for this bug: do the normal CSS link tag in non-build scenarios, documenting that it only works for CSS on the same domain, then have a build option that inlines the CSS with the dojo.requireCss call if you want xdomain support.

And/or have the build convert the CSS file into a .js file that can be loaded xdomain, and then take the contents and inject it into the page. That version of requireCss would require an extra param to indicate we are loading a JS file.

This would require a function, probably in loader.js (Dojo Base) that can take a string of CSS and append to head as a style tag, and fixing paths to images in the process.

It would be a lot easier/simpler/less code if FF would fix the bug. :p So much hoping for that.

Marking as future until actual work begins.

comment:11 Changed 11 years ago by bill

See also #5136.

comment:12 Changed 8 years ago by skaegi

I just noticed that https://bugzilla.mozilla.org/show_bug.cgi?id=185236 was fixed. Hoping there is a future again for something like requireCss...

comment:13 Changed 7 years ago by ben hockey

Keywords: needsreview added
Priority: highlow

comment:14 Changed 7 years ago by bill

Cc: Rawld Gill added
Component: CoreLoader
Description: modified (diff)
Keywords: reviewed added; needsreview removed
Summary: dojo.requireCss?dojo/css! plugin

We definitely still want a way to ensure that a widget's CSS is loaded before the widget is instantiated.

dojo/text! solves half the problem, but we should have something that additionally:

  1. ensures the CSS gets added to the document, and only added once (regardless of how many widgets depend on it)
  2. path correction? I don't think we are using any filter:alpha() though, so maybe we don't need that

So I'd still like to keep this open, and hopefully get it fixed one day.

See https://gist.github.com/1370465 for some code we may be able to use.

Last edited 7 years ago by bill (previous) (diff)

comment:15 Changed 7 years ago by bill

#15544 is a duplicate of this ticket.

comment:16 Changed 7 years ago by dylan

We've discussed the possibility of splitting this portion of xstyle out of that package and making it available as a Dojo Loader plugin...

comment:17 Changed 7 years ago by Thomas Bachem

Would love to see the relevant xstyle parts as a Loader plugin!

comment:18 Changed 6 years ago by dylan

Milestone: future2.0
Owner: changed from James Burke to Kris Zyp
Status: newassigned

Reassigning to Kris, since he's the maintainer of xstyle.

comment:19 Changed 6 years ago by Kris Zyp

I think this strategy sounds. I've been doing a lot of dev on xstyle lately, getting build stuff ready, and extensions, but once we are ready for us to break this out and put the CSS loading functionality in Dojo, I can certainly do so.

Changed 6 years ago by codeseo

Attachment: phlebotomy.png added

Changed 6 years ago by Slavon

Attachment: main.inc.php added

comment:20 Changed 5 years ago by Thomas Bachem

Would still love to see the relevant xstyle parts transformed into a Dojo plugin. Can I help to make that happen?

comment:21 Changed 5 years ago by bill

See also https://github.com/ibm-js/delite/blob/master/css.js. Ignoring the has("builder") blocks, which are for requirejs builds, this is a minimal implementation that leverages existing dojo code (i.e. Deferred). Not sure if it works on IE8 or not though.

comment:22 Changed 5 years ago by Thomas Bachem

It should work in IE 8, as it's using the same method as xstyle 0.2.1, which supports IE 6+: https://github.com/kriszyp/xstyle/blob/v0.2.1/core/load-css.js.

Btw, I would like to start a discussion around browser sniffing to detect onload support for <style> elements, as it's impossible to feature detect this (see comments in linked xstyle code), but IE 6+, FF 9+, Chrome 19+ and Safari 6+ seem to support it (http://pieisgood.org/test/script-link-events/). So we basically need the other method just for FF 3.5/3.6/4 and older Safaris...

comment:23 Changed 5 years ago by bill

IIUC "old safaris" includes many people's (android?) phones. That's why we don't just get rid of the fallback code path.

comment:24 Changed 5 years ago by Thomas Bachem

I agree, and FF 3.5/3.6/4 is also still around quite a lot. But in order to use the native onload events for the other 90% of the users out there, it seems like we need browser sniffing.

comment:25 Changed 4 years ago by dylan

Milestone: 2.01.12
Resolution: patchwelcome
Status: assignedclosed

With the evolution of CSS preprocessors, and the difficulty in making the build system do the right thing, I think the current consensus is that loading CSS via AMD is probably an anti-pattern.

That said, if someone wants to finish this up, we'll consider it for 1.12, so marking as patchwelcome.

Note: See TracTickets for help on using tickets.