Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/enhancement: replace "s with 's in texts #8

Open
ElectricFeet opened this issue Jan 5, 2020 · 9 comments
Open

Bug/enhancement: replace "s with 's in texts #8

ElectricFeet opened this issue Jan 5, 2020 · 9 comments

Comments

@ElectricFeet
Copy link

If you enter text in the text fields of the Opening Hours Shortcode Builder that contains " characters, these get fed into the shortcode, causing unpredictable results.

For example, by entering Location1: open <a href="tel:1234567">1234567</a> in the Open text, I created a shortcode of : [op-is-open set_id="1234" open_text="Location1: open <a href="tel:1234567"> 1234567</a>"].

The result in this case is that the Open text is ignored and the default text is output.

It took me a while to figure out what was going on, as the builder is not fully idiot-proof :-))) Of course the "s were conflicting with the shortcode syntax and the whole thing was therefore being ignored somewhere along the line.

I would suggest that the Opening Hours Shortcode Builder should swap any "s for 's when building.

@janizde
Copy link
Owner

janizde commented Jan 6, 2020

Ahh right, they need to be escaped with \" probably! Thanks for mentioning!

@ElectricFeet
Copy link
Author

You're welcome! Yes, I arrived at escaping in my template in the end as well.

Unfortunately, the nested "s are a problem in the documentation for the main plugin too.

For instance, in the Common attributes for all shortcodes, before_title is listed as <h3 class="op-{name}-title">, but if you put this string into the shortcode (for example, in order to change to <h4>, it barfs and ignores you because of the nested "s.

So while fixing the Shortcode Builder by escaping "s is indeed the answer, it might also be an idea to use single quotes everywhere in the documentation, so people don't copy paste (like I did) and input unescaped characters into the shortcode and wonder why it doesn't work.

@ElectricFeet
Copy link
Author

Hmm. This is more complicated. There are two major problems with the before_title and after_title attributes.

The first problem is in the use of html in the shortcode.

The WordPress Shortcode API page says:

[ The ] use of HTML is limited inside shortcode attributes

and, while support was extended slightly at one point, the position is (same page):

Full usage of HTML in shortcode attributes was never officially supported, and this will not be expanded in future versions.

The point being that getting action in WordPress core to fix the next problem will not be possible, as this usage isn't supported in the first place.

So on to the next problem: That the Gutenberg editor isn't handling these quotes properly either. This is more serious for the plugin syntax, not just the builder.

To give some examples:

Define some shortcodes in Gutenberg as follows:

test1: [op-holidays set_id="1409" title="test1" before_title="<h4 class="op-holidays-title">" after_title="</h4>"] — with double quotes

test2: [op-holidays set_id="1409" title="test2" before_title="<h4 class='op-holidays-title'>" after_title="</h4>"] — with single quotes

test3: [op-holidays set_id="1409" title="test3" before_title="<h4 class=\"op-holidays-title\">" after_title="</h4>"] — with escaped double quotes

test4: [op-holidays set_id="1409" title="test4" before_title="<h4 class=\'op-holidays-title\'>" after_title="</h4>"] — with escaped single quotes

Press Preview and you get: test1 and test3 ignore the h4 tag (and show the default h3 tag) and test2 and test4 show the h4 tag. So double quotes don't work at all, even if you escape them.

However, now things get worse. Now press update to save the page. No change is apparent. Now refresh the page.

The saved shortcodes are now retrieved and you can see that they have been modified by Gutenberg as follows:

test1: [op-holidays set_id="1409" title="test1" before_title="<h4 class="op-holidays-title">" after_title="</h4>"] — still has (failing) double quotes

test2: [op-holidays set_id="1409" title="test2" before_title="<h4 class="op-holidays-title">" after_title="</h4>"] — now has (failing) double quotes; Gutenberg has modified the previous (working) single quotes

test3: [op-holidays set_id="1409" title="test3" before_title="<h4 class="\&quot;op-holidays-title\&quot;">" after_title="</h4>"] — now has an inexplicable mess; goodness knows what Gutenberg has done here, but as it now has double quotes again, it fails

test4: [op-holidays set_id="1409" title="test4" before_title="<h4 class="\'op-holidays-title\'">" after_title="</h4>"] — still has the escaped single quotes, but Gutenberg has kindly wrapped them in double quotes so, again, it fails

So now you press Preview and all four fail.

It looks like a bug in Gutenberg to me, but given that HTML is not supported in shortcodes, there will probably be no appetite for fixing it.

A pragmatic solution would be to modify the shortcode syntax so that before_title/after_title become before_title_tag/after_title_tag & before_title_class/after_title_class, while before_title/after_title are deprecated.

In this scenario, anyone who is currently using the before_title syntax in a shortcode in the classic editor who edits the page with Gutenberg will in any case now have a failing shortcode — due to Gutenberg, not the plugin. They will need to modify the shortcode in any case, so they can modify it to the new syntax. (So this needs to be a sticky post on the wordpress support forum and in a FAQ on the wordpress.org plugin page). If they touch nothing — i.e. they don't save the page with Gutenberg — the deprecated code will still work for them behind the scenes.

Those using the widget (rather than the shortcode) will not have seen this problem, because it only allows you to change the class, not the tag.

Phew! Sorry that was so long. Hope it helps!

Ha! Last minute edit: [op-holidays set_id="1409" title="test5" before_title='<h4 class="op-holidays-title">' after_title='</h4>'] — with external single quotes and internal double quotes — works, so we have a workaround that doesn't get messed up by Gutenberg

@janizde
Copy link
Owner

janizde commented Jan 9, 2020

Thanks for all the research you did! The reason these attributes are in there is mainly because these are standard attributes of widgets (so the widget title looks same in every widget in a specific template). Behind the scenes every widget actually renders a shortcode which is why it's required to support it. I just looked and the widget rendering circumvents the shortcode as a string and directly calls renderShortcode:

https://github.com/janizde/WP-Opening-Hours/blob/981740b8d38020fb2388876893815dbfa8b960c9/classes/OpeningHours/Module/Widget/AbstractWidget.php#L160-L162

In v3 of the plugin I'm looking into how to make custom templating more convenient for users of the Plugin. Maybe I can then offer tools that make these shortcode attributes obsolete

@janizde
Copy link
Owner

janizde commented Jan 9, 2020

@ElectricFeet have you tried replacing " with ' in the html attributes instead of the shortcode attributes, as in

[op-overview before_widget="<h1 class='my-class-name'>"]

Then we wouldn't have to change it for everything in the shortcode. I would actually prefer to keep double quotes in the shortcode attributes and use single quotes as a special case for HTML in attributes

@ElectricFeet
Copy link
Author

Maybe some sort of Gutenberg block rather than a template? I'm new to Gutenberg, so that might be a stupid question!

@ElectricFeet
Copy link
Author

Re your question: yes, that's the test2 scenario above. Gutenberg changes them(!) to double quotes so that it fails.

@ElectricFeet
Copy link
Author

(N.B. all my tests were with before_title, not before_widget, but I don't think it makes any difference)

@ElectricFeet
Copy link
Author

@janizde p.s. There's also a an empty <p></p> before and after the heading when it works as well. That is, this shortcode:

[op-holidays set_id="1409" title="Prossime chiusure" highlight="true" date_format="j F" template="list" before_title='<h4 class="op-holidays-title">' after_title="</h4>"]

produces

<div class="op-holidays-shortcode">
<p></p>
<h4 class="op-holidays-title">Prossime chiusure</h4>
<p></p>
<dl class="op-list op-list-holidays">
        <dt class="">First hol</dt>
    <dd class="">11 Gennaio</dd>
        <dt class="">second hol</dt>
    <dd class="">13 Gennaio</dd>
        <dt class="">A loong weekend</dt>
    <dd class="">17 Gennaio - 20 Gennaio</dd>
    </dl>

</div>

I'm not sure where they come from. Maybe wpautop is getting to it somehow. It's not a major problem as I simply removed the margins on the <p>s with CSS and they are effectively invisible.

(Sorry to be the bringer of bad tidings all the time!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants