major upgrade lightbox and usergallery plugins

[yellowled]

And if those work as well, I believe we can drop the fixchrome.css altogether?

Well, we'd need to add said CSS to s9y's fallback styles, I guess. Which would make this another case of “only release the plugin update after an updated version of s9y has been released”, which is kind of a bummer. But I guess we can do that.

YL

[yellowled]

I'll leave this open for other people to confirm, and then I'll test it with the other lightbox scripts as well. And if those work as well, I believe we can drop the fixchrome.css altogether?

For the record, I just quickly tested this with prettyPhoto, ColorBox and ven jQuery Lightbox 2 – it does solve the issue for all of them.

YL

[Timbalu]

Great! Was this a matter of an updated webkit? Or does it even work with "older" webkit engined Browsers (lets say ~1 year old...)?

[yellowled]

Was this a matter of an updated webkit? Or does it even work with "older" webkit engined Browsers (lets say ~1 year old...)?

I have no older WebKit browser available, so I could not even test it.

It's not as if the issue is fixed just by an updated WebKit (or lightbox plugin, for that matter) – it still needs a CSS fix (in form of display: block on .serendipity_image_link), and while that is kind of similar to your previous fix, I assume this to be way less prone to error in relation to other (theme) styles just because of the fact that it does not generate a :before pseudo element or require a @media query.

So if you're asking “Will this work in older versions of Chrome or Safari?“ – I don't know. I don't consider it to be a problem in Chrome since it's an evergreen browser. Unfortunately, Safari is not. But since there is no good way to test that and no good way to only use a different fix for older Safaris (if that were necessary, which we don't know), I'd say we apply the new fix and wait for people to report errors (which I don't anticipate ever happening).

I would like to proceed as follows:

• add the fix to the lightbox plugin instead of the old fix in fixchrome.css for the time being
• add it to master in /templates/default/style_fallback.css so that we can remove fixchrome.css from the plugin once 2.1 is released

Does that seem reasonable?

YL

[Timbalu]

add it to master in /templates/default/style_fallback.css so that we can remove fixchrome.css from the plugin once 2.1 is released

No. We can't do this for compat. But we can write a condition for >= 2.1 to not use it.

[yellowled]

Personally, I think the best solution for this would be addToCSS – emit the fix if it's not in your theme's stylesheet, no matter which s9y version.

I'm not sure I can figure out how to implement that, but I'll try.

YL

[yellowled]

Personally, I think the best solution for this would be addToCSS

After looking at how that works, nope. addToCSS just checks if the class in question is defined in the CSS at all, it does not check if a certain property has as certain value. Too prone to error.

So, how do we do this switch?

Code: Select all

if ($serendipity['version'][0] == '1') {
    echo '    <link rel="stylesheet" type="text/css" href="' . $pluginDir . '/fixchrome.css" />' . "\n";
} else {
    if ($serendipity['version'][1] < '1') {
        echo '    <link rel="stylesheet" type="text/css" href="' . $pluginDir . '/fixchrome.css" />' . "\n";
    }
}

That seems incredibly clumsy. There's probably a better way to do this. I'll leave it to you.

YL

[Timbalu]

Code: Select all

if (strpos($eventData, '.serendipity_BlahBlah') === false) {
    $this->addToCSS($eventData);
}

or

Code: Select all

if (strpos($eventData, '.serendipity_BlahBlah') === false) {
    $eventData .= 'fake example for the styles you wish to append';
}

I normally do for 2.1 cases

Code: Select all

if (version_compare($serendipity['version'], '2.0.99', '<')) {
    $eventData .= 'fake example for the styles you wish to append';
}

or

Code: Select all

if (version_compare($serendipity['version'], '2.0.99', '<')) {
    echo '    <link rel="stylesheet" type="text/css" href="' . $pluginDir . '/fixchrome.css" />' . "\n";
}

But personally I try to avoid checks like that when possible, since that happens on each frontpage request - and each check, even when singulary fast on its own, sums down performance at the very end.

[yellowled]

But personally I try to avoid checks like that when possible, since that happens on each frontpage request - and each check, even when singulary fast on its own, sums down performance at the very end.

I can not measure or judge that. All I know is that referencing an external stylesheet for something that boils down to a single line of CSS is insane.

I'll commit the 4th example (thanks) to get this over with, the performance stuff is a separate discussion anyway.

YL

[Timbalu]

Ah - now I see what you were up for.
This is a single line - we should not add an extra file for this!
Can't we use that global for all browsers? And in the plugin only?
If you answer me this, I'll better add that before the sync.

[yellowled]

Can't we use that global for all browsers?

I have never tested this in any IE or an Android browser, but I assume so. I also assume it does not do any harm if the plugin is not installed, it may even make image links more clickable (because the link now spans the whole image, not just the bottom). That's why I would leave it in style_fallback.css either way.

If you answer me this, I'll better add that before the sync.

I assume you saw b6b0cc. If you can change that before the sync to use addToCSS, great. If not, no biggie (Personally, I only care if this works properly in > 2.1.)

YL

[Timbalu]

Done fd32bc8. Can you please test this with FF and without the 2.1 fallback commit?

[yellowled]

Can you please test this with FF and without the 2.1 fallback commit?

Sorry, I don't understand what you want me to test with what?

YL

[Timbalu]

test https://github.com/s9y/additional_plugi ... 24db739eb5 without the 2.1 fallback commit ...

[yellowled]

If the fix is in /templates/default/style_fallback.css, it is still emitted in serendipity.css by the lightbox plugin. I assume that is what you meant. If it is missing in style_fallback.css, it is still emitted by the plugin in serendipity.css.

YL