Permit use of HTML in attributes by encoding them them #496

mattheu · 2015-10-06T10:19:31Z

I picked up #322 - synced up with master, and made a few small changes.

The main change is that only the default value of 'escape' for a single attribute is taken from shortcodeUIFieldData[ type ] - this allows you to override on a single attribute basis. EG allowing HTML in a text field - not just textareas. Only textarea is true by default - and doesn't apply if this relates to 'inner_content'

Also added tests.

One question - Should we replace all use of the term 'escape' with 'encode' as we're not really escaping.

One hitch - Rob orignally wrote some clever code that uses the filter shortcode_atts_{$shortcode_tag} to unencode the value automatically. However this relies on a closure and hence is PHP 5.3+ only - and causes tests to fail. This isn't really avoidable - as the shortcode tag isn't available in the callback. I'm thinking of opening a ticket for this - but for now we need to either remove or allow breakage on PHP 5.2.

Adds html escaping in textareas

# Conflicts: # js/build/field-attachment.js # js/build/field-color.js # js/build/shortcode-ui.js # js/src/views/edit-shortcode-form.js

…attributes

danielbachhuber · 2015-10-06T13:54:46Z

js-tests/build/specs.js

@@ -58,7 +59,7 @@ var Shortcode = require('../../js/src/models/shortcode');
 var InnerContent = require('../../js/src/models/inner-content');
 var ShortcodeAttribute = require('../../js/src/models/shortcode-attribute');
 var ShortcodeAttributes = require('../../js/src/collections/shortcode-attributes');
-var $ = (typeof window !== "undefined" ? window['jQuery'] : typeof global !== "undefined" ? global['jQuery'] : null);
+var $ = (typeof window !== "undefined" ? window.jQuery : typeof global !== "undefined" ? global.jQuery : null);


Can you update your node modules and recompile?

mattheu · 2015-10-06T13:56:10Z

Updated

Updated all use of 'escape' to 'encoded' for consistency.
Fixed PHP 5.2 - changed the callback to use current_filter to determine the current shortcode from within the callback as reccommended here: https://core.trac.wordpress.org/ticket/15155#comment:12

danielbachhuber · 2015-10-06T14:01:43Z

One question - Should we replace all use of the term 'escape' with 'encode' as we're not really escaping.

Yes please.

Only textarea is true by default - and doesn't apply if this relates to 'inner_content'

For consistency and simplicity, can we just disable it by default for all arguments? I'm concerned with leaving it magically on, as it will always require developer intervention.

mattheu · 2015-10-06T14:33:08Z

Updated - no longer sets any defaults.

danielbachhuber · 2015-10-06T14:50:48Z

inc/class-shortcode-ui.php

+	 * @param  [type] $atts  [description]
+	 * @return [type]        [description]
+	 */
+	function filter_shotcode_atts_decode_encoded( $out, $pairs, $atts ) {


filter_shotcode_atts_decode_encoded should be filter_shortcode_atts_decode_encoded

Also, can we mark it explicitly as public ?

And, should we add tests for this method?

danielbachhuber · 2015-10-06T18:25:42Z

@goldenapples #reviewmerge

danielbachhuber · 2015-10-06T20:14:47Z

Oh. Should it be encoded or encode ?

danielbachhuber · 2015-10-06T20:21:31Z

I think I ran into a bug.

danielbachhuber · 2015-10-06T20:31:26Z

I think I ran into a bug.

If I add encoded=>true to an existing shortcode attribute, what's the expected behavior for all of the shortcodes I already have saved to the database?

mattheu · 2015-10-06T20:34:57Z

Oh. Should it be encoded or encode ?

You're right - should probably be encode. I was wondering this as I did it. I'll update.

goldenapples · 2015-10-06T21:41:08Z

If I add encoded=>true to an existing shortcode attribute, what's the expected behavior for all of the shortcodes I already have saved to the database?

Hmm. It's quite possible for field content which was legitimate before to break on urldecoding / re-encoding. Seems like there needs to be a migration path for this case. I don't know what to suggest that doesn't seem hacky and ugly, but allowing a developer changing a setting to corrupt all the existing data is a pretty big issue.

Could we somehow have a magical attribute which flags which fields have encoded data, that is set on formatting the shortcode? That way rendering the shortcode could read from that value when determining whether to urldecode content?

…nto html-in-attributes

mattheu · 2015-10-07T15:02:23Z

If you have unencoded data and want to enable encod, this will mostly just work OK. Except if you have added HTML that contains the following characters "'[] - in which case your shortcodes are broken already.

Pretty sure there is nothing we can do here as its most likely broken the PHP shortcode parsing too. Actually - side note - perhaps we should 'escape for shortcode attribute' and strip these characters? At least things won't break. I'll create a separate ticket.

If you have encoded data and want to disable, this will cause you to see encoded data and break things. You will need to run some kind of script. HOWEVER - note that your unencoded string will probably break the shortcode parsing.

I've had a quick go at writing a CLI command to handle this - however I'm wary that it appears more useful than it really is.

Permit use of HTML in attributes by encoding them them

A couple of field types were saved in separate files when this brnach was started, and are no longer. Deleted them and re-browserified the js/build files.

Rebuild `js/build` files after #496

Since Shortcake added support for URL-encoding attribute values (wp-shortcake/shortcake#496), we can remove the unfriendly admonitions to avoid using quote marks, branckets, or HTML in the alt and caption fields here.

roborourke and others added 14 commits May 7, 2015 01:03

adds html escaping functionality, first pass

8bd76a8

Merge pull request #1 from sanchothefat/html-in-attributes

89de6dc

Adds html escaping in textareas

fix phpcs warnings

0f62cc0

Merge branch 'master' into html-in-attributes

c7aada1

# Conflicts: # js/build/field-attachment.js # js/build/field-color.js # js/build/shortcode-ui.js # js/src/views/edit-shortcode-form.js

Handle escaped data

abaa232

Store escape logic on attribute model

08a75c7

Update dev file to handle new encoded attributes

19d6de2

Merge branch 'master' of github.com:fusioneng/Shortcake into html-in-…

da1c6b6

…attributes

recompile

ce13acd

encode/decode in format and parse methods

d8cd197

Tests for encoded attributes

903adb1

Use encoded not escaped as property

27b0f96

don't use closure for filter callback

3142e50

update docs and simplify code

1928826

danielbachhuber reviewed Oct 6, 2015
View reviewed changes

danielbachhuber changed the title ~~Html in attributes~~ Permit use of HTML in attributes by escaping them Oct 6, 2015

danielbachhuber changed the title ~~Permit use of HTML in attributes by escaping them~~ Permit use of HTML in attributes by encoding them them Oct 6, 2015

danielbachhuber added this to the v0.6.0 milestone Oct 6, 2015

danielbachhuber added the enhancement label Oct 6, 2015

mattheu added 2 commits October 6, 2015 15:04

recompile JS

2c7f809

don't set default to true

89f76bb

danielbachhuber mentioned this pull request Oct 6, 2015

Allow HTML in attributes #322

Closed

Don't set a default for encoded from shortcodeUIFieldData

1a98c85

danielbachhuber reviewed Oct 6, 2015
View reviewed changes

mattheu added 2 commits October 6, 2015 16:14

docs

e416daa

escape all the things

75d42d7

mattheu and others added 5 commits October 6, 2015 16:32

fix typo

111cacd

PHP test attribute decoding

09bd2b9

fix error if shortcode_atts called incorrectly

095963e

fix test

6d1b861

Merge branch 'master' into html-in-attributes

2906c29

danielbachhuber assigned goldenapples Oct 6, 2015

danielbachhuber added the review:engineering label Oct 6, 2015

danielbachhuber mentioned this pull request Oct 6, 2015

Tabs, not spaces, for indentation; use esc_html__() for escaping #497

Closed

khromov mentioned this pull request Oct 6, 2015

Textarea does not support linebreaks in parameters #485

Closed

mattheu added 2 commits October 7, 2015 10:01

Rename 'encoded' to 'encode'

586c55f

Merge branch 'html-in-attributes' of github.com:fusioneng/Shortcake i…

fb975d7

…nto html-in-attributes

mattheu mentioned this pull request Oct 7, 2015

Strip invalid characters from attributes when outputting. #500

Open

goldenapples added a commit that referenced this pull request Oct 12, 2015

Merge pull request #496 from fusioneng/html-in-attributes

a82cff8

Permit use of HTML in attributes by encoding them them

goldenapples merged commit a82cff8 into master Oct 12, 2015

goldenapples deleted the html-in-attributes branch October 12, 2015 19:27

This was referenced Oct 12, 2015

Problem with Quotation marks in text fields #342

Closed

urlencode or otherwise escape attributes when sending to editor #321

Closed

goldenapples added a commit that referenced this pull request Oct 12, 2015

Rebuild js/build files after #496

13e9c9e

A couple of field types were saved in separate files when this brnach was started, and are no longer. Deleted them and re-browserified the js/build files.

goldenapples mentioned this pull request Oct 12, 2015

Rebuild js/build files after #496 #510

Merged

goldenapples added a commit that referenced this pull request Oct 12, 2015

Merge pull request #510 from fusioneng/rebuild-after-496

58e90f8

Rebuild `js/build` files after #496

goldenapples mentioned this pull request Nov 23, 2015

Add "encode => true" to alt and caption fields wp-shortcake/image-shortcake#67

Merged

westonruter mentioned this pull request Jun 9, 2017

De-duplicate parsing logic across PHP and JS WordPress/gutenberg#1086

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Permit use of HTML in attributes by encoding them them #496

Permit use of HTML in attributes by encoding them them #496

mattheu commented Oct 6, 2015

danielbachhuber Oct 6, 2015

mattheu commented Oct 6, 2015

danielbachhuber commented Oct 6, 2015

mattheu commented Oct 6, 2015

danielbachhuber Oct 6, 2015

danielbachhuber Oct 6, 2015

danielbachhuber commented Oct 6, 2015

danielbachhuber commented Oct 6, 2015

danielbachhuber commented Oct 6, 2015

danielbachhuber commented Oct 6, 2015

mattheu commented Oct 6, 2015

goldenapples commented Oct 6, 2015

mattheu commented Oct 7, 2015

Permit use of HTML in attributes by encoding them them #496

Permit use of HTML in attributes by encoding them them #496

Conversation

mattheu commented Oct 6, 2015

danielbachhuber Oct 6, 2015

Choose a reason for hiding this comment

mattheu commented Oct 6, 2015

danielbachhuber commented Oct 6, 2015

mattheu commented Oct 6, 2015

danielbachhuber Oct 6, 2015

Choose a reason for hiding this comment

danielbachhuber Oct 6, 2015

Choose a reason for hiding this comment

danielbachhuber commented Oct 6, 2015

danielbachhuber commented Oct 6, 2015

danielbachhuber commented Oct 6, 2015

danielbachhuber commented Oct 6, 2015

mattheu commented Oct 6, 2015

goldenapples commented Oct 6, 2015

mattheu commented Oct 7, 2015