I am not sure if this is intentional or not. CSS defined by markitup override theme CSS.

For example, I am testing markitup for a site, and trying to alter the width of the editor window from the default of 700px, which is defined in wysiwyg>markitup>markitup>skins>simple>style.css

Aside from editing module>plugin files, can this be worked around?

thx

CommentFileSizeAuthor
#21 drupal.css-basename.21.patch1.64 KBsun
#20 drupal.css-basename.20.patch1.62 KBsun
#19 cssbasename.patch1.48 KBcasey
#12 cssbasename.patch1.12 KBcasey

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Closed (works as designed) 
function wysiwyg_markitup_settings($editor, $config, $theme) {
  // Whoever is guilty for adding this horrible CSS-file-without-filepath
  // override "feature" to Drupal core... stand in the corner!
  drupal_add_css($editor['library path'] . '/skins/' . $theme . '/style.css', 'theme');
...

For now, this is by (Drupal core) design. It's a mess, but I can't help, sorry.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: markitup CSS override theme CSS » Theme system prevents loading "style.css"
Project: Wysiwyg » Drupal core
Version: 6.x-1.1 » 7.x-dev
Component: Editor - markItUp » theme system
Category: support » bug
Priority: Normal » Critical
Status: Closed (works as designed) » Active

No. I don't want this to vanish. This is a critical Drupal core _bug_. D7 can't be released without fixing it.

I hate magic.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

This issue gets even more important when considering support for external CSS...

avolve’s picture

avolve commented

Thanks for the quick follow up and outlining the theme function — much appreciated!

caktux’s picture

caktux commented
Project: Drupal core » Wysiwyg
Version: 7.x-dev » 6.x-1.1
Component: theme system » Editor - markItUp

setting back as Wysiwyg API issue, I'm pretty sure this isn't a D7 Core issue if wysiwyg_markitup_settings() has style.css hardcoded in it.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Project: Wysiwyg » Drupal core
Version: 6.x-1.1 » 7.x-dev
Component: Editor - markItUp » theme system

Don't do that again.

dvessel’s picture

dvessel commented
Title: Theme system prevents loading "style.css" » Refactor style sheet overrides.
Priority: Critical » Normal

I'm not sure this is documented but every single styles sheet must have unique names. This goes beyond style.css. If any theme defined style collides with a module, it'll go missing as it was designed to do.

It can be improved but I haven't seen this encountered very often. If there's a patch I'd be happy to review. Otherwise, your editor will never get fixed.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: Refactor style sheet overrides. » Stylesheet overrides prevent loading of certain filenames
Priority: Normal » Critical

Yes, the design is wrong. The CSS filename in question is not under our own control. Users are asked to download that editor, extract it, and be happy. Drupal makes them unhappy.

This is critical.

avolve’s picture

avolve commented

Digging through CSS i found that the width of the editor window is governed for nodes by 

.node-form textarea {
  display: block;
  width: 95%;
}

in node.css

For comments, I added a 

#comment-form textarea {
  display: block;
  width: 90%;
}

to the custom-theme.css. Whilst a workaround, it resolves this issue.

[the 90% as opposed to 95% was required by the custom theme. Width can also be set as px, em, etc]

casey’s picture

casey commented

@sun what exactly is going wrong here?

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: Stylesheet overrides prevent loading of certain filenames » Stylesheet override logic prevents loading of stylesheets of third-party libraries

Better title.

What's going wrong?

1) Put the "library A" into your Drupal installation, which ships with a "style.css".

2) Presuming that you have Garland or any other theme enabled that's using "style.css",

3) Try to load library A's stylesheet.

4) Alright, you managed to do 3) using some weird hacks, but now allow the theme to override the styles defined in library A's style.css.

casey’s picture

casey commented
Status: Active » Needs review
StatusFileSize
new cssbasename.patch1.12 KB

Initial patch. Allows to do the following.

$options = array(
  'basename' => $editor['library name'] . '-style.css'
);
drupal_add_css($editor['library path'] . '/skins/' . $theme . '/style.css', $options);
sun’s picture

sun
Primary language German
Location Karlsruhe
commented

That's quite a ugly hack, but it could work.

casey’s picture

casey commented

oww I kinda liked it :p

sun.core’s picture

sun.core commented 
+++ includes/common.inc	24 Nov 2009 01:35:19 -0000
@@ -3130,6 +3130,7 @@
+ *   - 'basename': @todo.

Can we resolve this and get this done?

Powered by Dreditor.

sun.core’s picture

sun.core commented

#12: cssbasename.patch queued for re-testing.

sun.core’s picture

sun.core commented
Component: theme system » base system

Status: Needs review » Needs work

The last submitted patch, cssbasename.patch, failed testing.

casey’s picture

casey commented
Status: Needs work » Needs review
StatusFileSize
new cssbasename.patch1.48 KB

Reroll

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new drupal.css-basename.20.patch1.62 KB

Thanks, slightly reworded the comments, RTBC.

@casey: Please make sure you roll your patches with diff -up

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal.css-basename.21.patch1.64 KB

Upon reading it once more, clarified some more. ;)

sociotech’s picture

sociotech commented

In the process of porting Fusion to D7 I ran across this problem, since sub-themes can have css files with the same name as the base theme, but they are meant to _add/override_ their css to the base theme's css, rather than _replace_ it.

So, I applied this patch and tested it out. Works great when using the theme name as a prefix in the basename option.

Thumbs up and thanks very much for this handy bug fix.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.