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

FIX: Fixes #2382 #2417

Closed
wants to merge 1 commit into from
Closed

FIX: Fixes #2382 #2417

wants to merge 1 commit into from

Conversation

phptek
Copy link

@phptek phptek commented Sep 13, 2013

  • Errors are thrown in front-end should the shortcode parser encounter a shortcode with a value of '0'
  • Added test

@phptek
Copy link
Author

phptek commented Sep 13, 2013

Best get this reviewed by @hafriedlander I suspect.

- Errors are thrown in front-end should the parser encounter a shortcode with a value of '0'
- Added test
@hafriedlander
Copy link
Contributor

Looks good. Won't work for [test_shortcode foo=""] though. Think that's a problem? If so, off the top of my head:

$attr = array_values(array_filter($attr, 'strlen'));
$list = array_shift($attr);
$name = array_shift($attr);
$value = $attr ? array_shift($attr) : '';

@phptek
Copy link
Author

phptek commented Sep 13, 2013

Yeah I had tests for that case also, which also failed prior to applying fix. I didn't consider it directly related to the current ticket, hence my not including them.

@hafriedlander I will change as per one of the above and add tests accordingly.

@halkyon
Copy link
Contributor

halkyon commented Oct 31, 2013

@phptek Any further updates for this? would be good to merge this one :)

@phptek
Copy link
Author

phptek commented Nov 1, 2013

@halkyon interesting. Just checked out latest 3.1, reverted my fixes and the issue no longer seems to arise:

I performed the following (rough and ready test)

  • Create a link in the CMS to a known page e.g. "Contact us"
  • Go into TinyMCE's HTML view and modify sitetree_link,id=3 to become sitetree_link,id=0 (or sitetree_link,id=
    0' or even as @hafriedlander pointed out sitetree_link,id="")
  • Save and Publish
  • Voila - no error, but the link is highlighted as invalid - presumably.

Nofix?

@simonwelsh
Copy link
Contributor

Since the error this fixes can no longer be reproduced, I'm closing this.

@simonwelsh simonwelsh closed this Mar 15, 2014
@td204
Copy link

td204 commented Nov 3, 2015

Hi there! This issue is still around in 3.1.15. The fix supplied works fine for me.
Note that the issue only arises when the environment type is set to 'dev' or 'test', since it throws a PHP Notice.

@sheadawson
Copy link

This is an issue for me also

@tractorcow
Copy link
Contributor

What kind of shortcodes are you using? Can you please give examples that would be useful to reproduce your error?

@sheadawson
Copy link

Using my shortcodable module, checkbox field values used to create shortcode attributes are 1 or 0. I could change it to use true/false strings but that seems like a work around for a core bug that should peobably be fixed?

[TrailforksWidget trailid="28447" map="1" elevation="1" photos="0"]

@michalkleiner
Copy link
Contributor

Still an existing issue, would be good to merge the pull request with the fix for this.
My shortcode looks like: [ItemType color="0"]

@tractorcow
Copy link
Contributor

See #2382 (comment)

If someone wants to back-port this fix to 3.x then I'll review any PR. :)

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

Successfully merging this pull request may close these issues.

8 participants