Opened 7 years ago

Closed 7 years ago

#15434 closed defect (fixed)

When using themeMap for user supplied custom theme *-compat.css file not loaded

Reported by: Bill Reed Owned by: Eric Durocher
Priority: high Milestone: 1.8
Component: DojoX Mobile Version: 1.7.2
Keywords: Cc: Adam Peller, cjolif
Blocked By: Blocking:

Description

We have run across this issue in Maqetta using a users modified custom theme with dojox.mobile widgets

deviceThemes and _compat.js only load *-compat.css files from dojox/mobile/theme*

Use Case:

The user copies the dojox/mobile/theme/custom to a folder in their workspace and modifies that theme for their company.

The user then attempts to use the themeMap in their HTML file to specify the new custom theme.

If the user runs the HTML file in a non webkit browser the themes comapt files are not loaded because the theme does not reside in dojox/mobile/theme folder.

I have attached a zip file that contains the sample html file and the custom theme. I have also attached a patch file with a proposed fix to dojox/mobile/_compat.js

Attachments (3)

compat_test.zip (210.0 KB) - added by Bill Reed 7 years ago.
zip file so use case
_compat.js.patch (800 bytes) - added by Bill Reed 7 years ago.
proposed patch
patch15434.patch (1.2 KB) - added by Adrian Vasiliu 7 years ago.
Adrian Vasiliu, IBM, CCLA - allows to use a custom theme located outside a mobile/themes directory

Download all attachments as: .zip

Change History (16)

Changed 7 years ago by Bill Reed

Attachment: compat_test.zip added

zip file so use case

Changed 7 years ago by Bill Reed

Attachment: _compat.js.patch added

proposed patch

comment:1 Changed 7 years ago by Adam Peller

Cc: Adam Peller added
Summary: When using themeMap for user supplied custom theme *-comapt.css file not loadedWhen using themeMap for user supplied custom theme *-compat.css file not loaded

comment:2 Changed 7 years ago by cjolif

Cc: cjolif added

comment:3 Changed 7 years ago by Adrian Vasiliu

The suggested patch has the drawback that it tries to import compat files not only for the theme's css but for any css loaded using a <link> in the html.

We will search for the ideal way to allow placing the custom theme in any directory structure.

That said, the ticket description says "compat files are not loaded because the theme does not reside in dojox/mobile/theme folder", but in fact, as you may have seen, the current constraint is to place the custom theme in dojox/mobile/themes in any directory containing "mobile/themes". So your test code loads the compat css just fine by modifying the directory structure as follows:

mobile
   themes
      common
      mycustom
compat_test.html

instead of the current structure which is:

mycustom
compat_test.html

Note that you also need a copy of dojox/mobile/themes/common. The reason is that some css files in your "mycustom" theme, just as in dojox/mobile/custom, import css files from dojox/mobile/common (using a ../common relative path). This is why "common" needs to be present. Of course, if a custom theme does not use any css from "common", this is not needed.

Would this solution be good enough for Maqetta at this stage?

Last edited 7 years ago by Adrian Vasiliu (previous) (diff)

comment:4 Changed 7 years ago by Bill Reed

Our current folder structure for cloning themes copies the css files to a folder in the users work space with the name of themes. So we do not have the ability to add mobile to the path.

project1
   themes
     common
     mytheme
   compat_test.tml

As for the drawback of the patch, that is not the way I understand the code.

deviceTheme.loadDeviceTheme function is called when the page is loaded deviceTheme.loadDeviceTheme() uses dm.loadedCssFiles to keep track of the css files loaded by loadDeviceTheme for unload if deviceTheme.loadDeviceTheme is called at runtime to switch user agents. We had this code added for Maqetta to enable dynamic device switching. The theme files (if any) for the last device are unloaded and then the theme files for the new device are loaded and save in the dm.loadedCssFiles. The only objects in dm.loadedCssFiles are links for the css mobile theme files that were loaded by deviceTheme.loadDeviceTheme.

deviceTheme.js about line: 251

//remove old css files
for(var k = 0; k < dm.loadedCssFiles.length; k++){
   var n = dm.loadedCssFiles[k];
   n.parentNode.removeChild(n);
}
dm.loadedCssFiles = [];
for(j = 0; j < files.length; j++){
   this.loadCssFile(files[j].toString());
}

I added added a link to app.css in my compat_test.html file to verify and indeed this is correct. When my patch gets called the only thing in dm.loadedCssFiles is a link to mycustom.css

The patch only loads -compat.css files from the theme loaded my deviceTheme not all links in the head

comment:5 Changed 7 years ago by Bill Reed

for clarity here is the code from deviceTheme.js for loadCssFiles()

this.loadCssFile = function(/*String*/file){
			// summary:
			//		Loads the given CSS file programmatically.
			var link = win.doc.createElement("link");
			link.href = file;
			link.type = "text/css";
			link.rel = "stylesheet";
			var head = win.doc.getElementsByTagName('head')[0];
			head.insertBefore(link, head.firstChild);
			dm.loadedCssFiles.push(link);
		};

comment:6 Changed 7 years ago by Adrian Vasiliu

Indeed, I wrongly said it would fail for a <link> to any css. But it would fail for a <link> to a theme css, which is a use-case that we do support. In your compat_test.html, replacing the loading of deviceTheme.js by, for example

<link href="<relativepath>/dojox/mobile/themes/iphone/iphone.css" rel="stylesheet">

the head of the page contains as expected a <link> to dojox/mobile/themes/iphone/iphone-compat.css. But this is missing if we'd use the suggested patch. (More exactly, using the patch as-is produces an error because it uses dm.loadedCssFiles which is undefined in this use-case. Modifying the patch to protect against this error helps, but this doesn't solve the missing link to iphone-compat.css.) The same issue exists if the html loads your custom theme using <link>.

We will come back with an alternate solution.

comment:7 Changed 7 years ago by Bill Reed

I agree my patch does need to test for dm.loadedCssFiles, my assumption was that if the user is using a link to include the theme instead deviceTheme they need to manage the addition of compat.css files on their own by add a script to add compat .css

Changed 7 years ago by Adrian Vasiliu

Attachment: patch15434.patch added

Adrian Vasiliu, IBM, CCLA - allows to use a custom theme located outside a mobile/themes directory

comment:8 Changed 7 years ago by Adrian Vasiliu

So the solution we implemented in the attached patch15434.patch consists in allowing the application to specify a custom pattern against which the css files are matched for deciding whether compat files are loaded. In the case of your compat_test.html:

<script type="text/javascript" src="...path to dojo.js..." 
  data-dojo-config="mblLoadCompatPattern: /\/mycustom\/.*\.css$/, async: true, parseOnLoad: true"></script>

We will also add a bit of documentation about this configuration parameter. We hope this fits your needs. Eric and Adrian.

Last edited 7 years ago by Adrian Vasiliu (previous) (diff)

comment:9 in reply to:  8 Changed 7 years ago by Bill Reed

Replying to Adrian:

So the solution we implemented in the attached patch15434.patch consists in allowing the application to specify a custom pattern against which the css files are matched for deciding whether compat files are loaded. In the case of your compat_test.html:

<script type="text/javascript" src="...path to dojo.js..." 
  data-dojo-config="mblLoadCompatPattern: /\/mycustom\/.*\.css$/, async: true, parseOnLoad: true"></script>

We will also add a bit of documentation about this configuration parameter. We hope this fits your needs. Eric and Adrian.

Adrian, I am don't think this solution will work for us, here is an example use case:

The user has created custom themes for multiple devices so their themeMap looks like:

data-dojo-config="'themeMap':[['Android',null,['myandroid/myandroid.css']],['BlackBerry',null,['myblackberry/myblackberry.css']],['iPad',null,['myipad/myipad.css']],['iPhone',null,['myiphone/myiphone.css']],['.*',null,['mycustom/custom.css']]],'mblThemeFiles':[]"></script>

In a case where the theme is changed based on device what would the mblLoadCompatPattern look like?

comment:10 Changed 7 years ago by Adrian Vasiliu

With all Maqueta custom themes being located in a "themes" directory as you described,

data-dojo-config="mblLoadCompatPattern: /\/themes\/.*\.css$/, ...

should work just fine. Alternatively:

data-dojo-config="mblLoadCompatPattern: /\/myandroid\/.*\.css$|\/myblackberry\/.*\.css$|\/mymyipad\/.*\.css$|\/myiphone\/.*\.css$|\/mycustom\/.*\.css$/, ...
Version 1, edited 7 years ago by Adrian Vasiliu (previous) (next) (diff)

comment:11 in reply to:  10 Changed 7 years ago by Bill Reed

Replying to Adrian:

With all Maqueta custom themes being located in a "themes" directory as you described,

data-dojo-config="mblLoadCompatPattern: /\/themes\/.*\.css$/, ...

should work just fine. Alternatively:

data-dojo-config="mblLoadCompatPattern: /\/myandroid\/.*\.css$|\/myblackberry\/.*\.css$|\/mymyipad\/.*\.css$|\/myiphone\/.*\.css$|\/mycustom\/.*\.css$/, ...

Semantically, the flag is a pattern which tells whether a path is the path of a theme css. So if there are different directories for different devices, you can indicate the list of all locations with a regexp "or" operator, as in the example above.

That should work for us, Thanks

comment:12 Changed 7 years ago by Adam Peller

Milestone: tbd1.8
Priority: undecidedhigh

comment:13 Changed 7 years ago by cjolif

Resolution: fixed
Status: newclosed

In [28731]:

fixes #15434. Allows to use a custom theme located outside a mobile/themes directory. Thanks Adrian Vasiliu, IBM, CCLA. !strict.

Note: See TracTickets for help on using tickets.