WordPress.org

Make WordPress Core

Recent Updates Page 4 Toggle Comment Threads | Keyboard Shortcuts

  • Joe McGill 7:14 pm on May 9, 2016 Permalink |
    Tags: ,   

    Media bug scrub summary, May 6, 2016 

    Last week, we held a media component bug scrub in #core-images. You can read the archive of the discussion here: https://wordpress.slack.com/archives/core-images/p1462561213000210

    Participants included: @joemcgill, @kkoppenhaver, @ocean90, @markoheijnen

    We reviewed all of the Media component tickets currently slated for the 4.6 release (6 in all), and began working through issues on the Awaiting Review milestone.

    Tickets reviewed

    • #36531 Default image size medium_large is not generated
    • #36316 Image editor: improve form validation errors handling
    • #36587 PHPUnit Tests fail with PHP7 and Imagick 3.4.x
    • #14244 Upload file types should be checked BEFORE uploading.
    • #17262 wp_get_attachment_thumb_file should check new ‘thumbnail’ image size
    • #34384 No way of preventing image_get_intermediate_size from returning cropped image
    • #36777 Grid view not working after upgrading to WordPress v4.5.1
    • #36735 Add media, insert from URL and alt attribute (marked as good first bug)
    • #36684 Image crunched does not render in Microsoft Edge/IE/Windows Photos

    We’ll pick this back up next week at the same time, Friday, May 13 at 19:00 UTC.

     
  • Dominik Schilling (ocean90) 3:46 pm on May 9, 2016 Permalink |
    Tags: ,   

    This Week in 4.6: May 9 – May 15 

    This is the jump-start post for the third week of the WordPress 4.6 release cycle.

    Priority Tickets:

    We’re early in the release cycle, and as such it’s time to to triage the tickets with the “early” tag. Those can be found in the “Next Release” section in this report: https://core.trac.wordpress.org/tickets/early

    Additionally, the following tickets should get some attention too. Each ticket proposes to introduce a new class:

    Each proposal should get evaluated whether it’s valuable for core or not. Some are already in a state where feedback and/or testing is required. If you’re interested in one of the classes please comment on the ticket.

    Meetings This Week:

    Bug Scrubs

    4.6 Feature Chats

    Weekly Chats

    Notable updates from last week:

     
  • Joe Hoyle 1:32 pm on May 6, 2016 Permalink |
    Tags: imagick,   

    ImageMagick Vulnerability Information 

    A few days ago an ImageMagick vulnerability was disclosed dubbed “ImageTragick” that affects WordPress websites whose host has ImageMagick installed. If you control your own hosting for your WordPress site, you should look to implement the following fix(es) immediately.

    The full effect of this issue is still unfolding and it’s not clear what the full impact will be; however, there are mitigation steps that should keep you safe with the known exploits so far. This vulnerability is already being exploited in the wild, and proof of concepts are being published.

    How is WordPress affected?

    Uploaded images to the WordPress admin that contain malicious data can perform a number of exploits via the underlying ImageMagick library. Uploads are available to all users who have the upload_files capability. By default that’s authors, editors, and administrators; contributors, subscribers, and anonymous users do not have that capability. This issue is still developing; however, it should be noted that if un-patched, this exploit allows for Remote Code Execution (RCE).

    What steps is the WordPress Core Team taking to mitigate this?

    The exploit is in the Imagick PHP extension, not WordPress itself (or any library that is shipped with WordPress). The WordPress Security Team is exploring ways to help mitigate this exploit due to the wide usage of ImageMagick in the WordPress ecosystem; however, this exploit is best handled at the hosting level (instructions below).

    How do I know if my site is vulnerable?

    If you do not control your own hosting environment then it’s likely that your host is taking steps to fix the issue. We recommend you reach out to your hosting provider to verify they are handling the “ImageTragick (CVE-2016-3714, CVE-2016-3718 and CVE-2016-3715)” exploit.

    Only WordPress sites that have the PHP Imagick extension installed are vulnerable to this exploit. If you don’t know if your server has this PHP extension, there are a few ways to identify this:

    1. Inspect the output of the phpinfo() function for “Imagick”.
    2. Run php -m | grep imagick on the command line.

    How do I patch the vulnerability?

    Currently the best known fix is to add a policy.xml file to your ImageMagick installation to limit the delegates that ImageMagick will use. Due to the ongoing nature of this issue, we recommend you refer to and follow https://imagetragick.com/ for instructions on how to handle the problem.

    ImageMagick 6.9.3-10

    ImageMagick released an update on 2016-05-03 to fix this issue; however, there are questions around whether this update provides a complete fix. At the time of writing it should be presumed version 6.9.3-10 does not fix the issues completely and you should take steps to patch the issue via the policy.xml file.

    Further reading

    More information and updates on this exploit can be found at https://imagetragick.com/. We recommend you keep an eye out for updates to this issue, as the full extent of problem is still being discovered.

    Documentation on the policy.xml file can be found at https://www.imagemagick.org/script/resources.php.

     
    • Brad Touesnard 7:11 pm on May 6, 2016 Permalink | Log in to Reply

      I just thought of another way WordPress sites might be affected. If your WordPress site is automatically pulling in public images and adding them to the Media Library. For example, your site may be pulling all tweets with a certain hashtag and loading the images into the Media Library. An attacker might be able to place an image to be pulled in.

      • Jeff Chandler 7:52 pm on May 6, 2016 Permalink | Log in to Reply

        This also occurs if someone is using PressThis to publish an article as it takes the image from the post and uploads it into the media library to avoid hotlinking.

    • Maeve Lander 5:13 am on May 9, 2016 Permalink | Log in to Reply

  • voldemortensen 8:05 pm on May 5, 2016 Permalink |
    Tags: , ,   

    Dev Chat Summary – May 4, 2016 

    This post summarizes the dev chat meeting from May 4.

    Update on WordPress 4.5.2

    • Lead for 4.5.2 still undecided.
    • 8 potential tickets for 4.5.2, only one has been committed.
    • #36534 is a serious issue that has had little activity. The issue is a bug between Imagick and OpenMP in certain configurations. @mike has reached out to a few major hosts who have fixed their configuration. @joemcgill and team are continuing the investigation into how to handle these errors.
    • The goal is to have as many 4.5.2 tickets committed to trunk by next week as possible.

    Triage of tickets with the “early” keyword

    Call for designers/reviving the chats

    • Since the beginning of this cycle a few contributors requested help from the team in #design. This channel has currently 813 members but the responses are only sporadic. There is also a meeting scheduled which should happen every week on Thursday at 17:00 UTC. But last one was in January.
    • @karmatosed and @hugobaeta will take charge of reviving chats in #design on Slack.
    • @hugobaeta will have an agenda posted by end of day today.
    • @zetaraffix volunteered to join the design team.

    Feature Project Updates

    Shiny Updates

    Fields API

    Customizer

    • Recap post of the last office hours: https://make.wordpress.org/core/2016/05/04/customize-office-hours-recap-2016-05-02/
    • #30937 (transactions) and #35210 (notification area) are the highest priorities.
    • A make/core post for Customizer setting validation (#34893) in the works.
    • @westonruter has also initiated a new feature plugin “JS Widgets” that will greatly improve the performance of widgets in the Customizer. An alpha state plugin can be found here: https://github.com/xwp/wp-js-widgets

    Decision time for: #36753 Use system fonts for a faster, more native-feeling admin (Font Natively)

    • Background information and rationale can be found here: https://make.wordpress.org/core/features/font-natively/
    • @helen mentioned that there is a patch on #36753 and that lots of screenshots across OSes and window sizes of current state and new state are needed. Help is appreciated.
    • Because this project is already in a committable state, @helen proposed to commit this patch now.
    • Even the final decision time is still a month away no objections were called.
    • @helen will commit the patch.

    Discussion: How to name new functions? get_sites/networks() vs wp_get_sites/networks()

    This came up during the last multisite chat. @jeremyfelt gave a short introduction:

    WP_Site_Query should have a function similar to get_posts(), get_users(), or get_comments() that allows for the simple return of results from a query.

    wp_get_sites() was introduced quite a bit ago. It returns an array of arrays rather than the desirable array of WP_Site objects.

    get_sites() is proposed as part of the WP_Site_Query patch to align better with the naming of posts, users, comments… and to allow for the return of an array of objects.

    We need to make a decision: Does it make sense to introduce get_sites() and deprecate wp_get_sites()

    I guess a broader question is do we push for more wp_ prefixed functions in situations like this or do we go with what we’ve been doing.

    A similar question exists for get_networks()

    • @tfrommen objects, that introducing a new unprefixed function might be a collision with some (also unprefixed) plugin function. He proposes that the slurping the plugin repo would be a good idea.
    • @boonebgorges states a rule: Prefer prefix as a rule of thumb, but exceptions seem fine in cases like this, when DX will be improved by being internally consistent.
    • @helen: Generally I would base these kinds of decisions on what wouldn’t trip you up (or at least you trip you up less) as a developer.
    • @ipstenu will help with slurping the plugin repo for get_sites() and report the findings in #core-multisite.
    • There is currently not a consensus on prefixing functions with wp_.

    Component Updates

    Taxonomy

    Multisite

    Comments

    • Bug scrub scheduled for Monday May 9th, 2016 at 13:00 MDT in #core Slack.
    • Watch for a Make/Core post regarding Custom Comment Types
    • With 104 open tickets, the Comments team would love help confirming bugs still exist, are actually bugs, or patches 🙂
    • #33717 (Send Notification Email When a Comment is Approved From Moderation) needs some extra review and feedback.

    Media

    • The media component could use a lot of love, and a bug-fix focussed release is just the sort of thing that could help.
    • @joemcgill  is planning to put out a call for volunteers on the make/core blog and then begin using our weekly dev chat time to focus on bug-scrubs.
    • There are currently 268 open tickets in that component, so extra hands will be helpful.

    Open Discussion

    • @helen brought a concern about removing the Open Sans style registration and people who might be using it as a dependency, etc. The decision was to leave the registration and move the line to the // Deprecated CSS section..
    • @ronalfy asked for a new owner of #33932 (Filters for Plugin/Theme Update Email Notifications).
    • @zstepek brought #34848 (Add support for updating post meta in bulk) up. @boonebgorges will be responding to the ticket soon.
    • @spacedmonkey would like to get more eyes on #35381 (Introduce WP_Term_Query)

     

    The full dev chat logs can be found here: https://wordpress.slack.com/archives/core/p1462392004003183

     
  • Joe McGill 6:48 pm on May 5, 2016 Permalink |
    Tags: ,   

    Media Component: Call for volunteers 

    The media component is a very large and highly-used component in WordPress—including sub-components like media uploading, the media editor, galleries, embeds, media template functions, etc. Maintaining such a large part of the codebase is a formidable task, and frankly, one that could use some extra love.

    As of this writing, there are 268 open tickets attributed to the media component, many of which haven’t received much attention. A release focused on bug-fixes is a perfect opportunity to reverse this trend.

    Get involved

    The team around the media component has gotten a bit light over the past few releases. If you’re interested in assisting with this effort, leave a comment on this post or reach out in the #core-images channel on Slack.

    We could use help on everything from reproducing bugs and verifying patches, to writing patches and unit tests. If you’re looking for a way to get involved in core development, this is a great place to get started.

    Weekly bug scrubs

    During the 4.6 release cycle, we’re going to experiment with using our weekly meeting times to do bug scrubs on the component in order to get the number of open bugs down to a reasonable number. The next meeting will be May 6 at 19:00 UTC in the #core-images channel on Slack.

     
  • Boone Gorges 5:15 pm on May 5, 2016 Permalink |
    Tags: , ,   

    Taxonomy bug scrub summary, 2016-05-05 

    We had a taxonomy component bug scrub today. Slack archive: https://wordpress.slack.com/archives/core/p1462464023003714

    Present: @boonebgorges, @helen, @barryceelen, @wonderboymusic. @swissspidy and @jans-1 voiced their opinions via emoji reactions.

    We cleared 8 items from the Awaiting Review queue, bringing it down to 35 tickets.

    Let’s do this again real soon!

     
  • Grant Palin 2:38 am on May 5, 2016 Permalink |
    Tags: ,   

    Week in Core, April 26 – May 3 2016 

    Welcome back the latest issue of Week in Core, covering changes [37314-37354]. Here are the highlights:

    • 40 commits
    • 33 contributors
    • 78 tickets created
    • 8 tickets reopened
    • 70 tickets closed

    Ticket numbers based on trac timeline for the period above.

    Code Changes

    Administration

    • improve the Star Ratings hiding empty elements for assistive technologies. [37330] #36725
    • This patch assigns the background color to body instead of the html element. [37321] #35314

    Build/Test Tools

    • Include npm prune in the before_script command. npm prune removes extraneous packages so the cache contains only current modules. [37340] #36490
    • Document WP_UnitTestCase->go_to() [37319] #36679

    Comments

    • date_query should be a property on WP_Comment_Query objects, nstead of a local variable. [37354] #36741
    • Realign parameter documentation in the DocBlocks for comment_author_email_link() and get_comment_author_email_link(). Also adds a missing return description for get_comment_author_email_link(). [37349] #36571
    • Adjust comment_author_email_link() and get_comment_author_email_link() to each accept a new optional fourth parameter, $comment, which enables overriding the $comment global. Adds tests. [37348] #36571
    • Display the comment counts in wp_dashboard_right_now() in the rare initial condition when there are 0 approved comments and only pending comments, so the AJAX count update could work. [37335] #35519
    • Pass $comment to comment_text() in Walker_Comment::comment() instead of using a function which can skip the cache. [37325] #35433

    Customize

    • Handle filtering sidebars_widgets when the underlying option is non-existent. See #36389. Fixes #36660. [37352] #36389, #36660
    • Pass WP_Customize_Setting instance as second argument to customize_value_{$id_base} filter. [37350] #36452
    • Allow Esc key to collapse the currently-expanded panel, section (or control). [37347] #22237
    • Ensure settings modified during an open save request remain dirty when save request completes. [37346] #32941
    • Increase the target size of the expand/collapse button in the customizer. [37341] #36093
    • Don’t auto-close the customizer when a new theme is activated. [37339] #35320
    • Remove format placeholders from widget templates and selectors, fixing a jQuery selector syntax error and the broken highlight/shift-click behaviors. [37322] #36473

    Database

    Docs

    • Improve the class DocBlock for WP_Widget to clarify which methods “should” vs “must” be overridden by extending sub-classes. [37343] #36703
    • Remove inline @see tags from function, class, and method references in inline docs. [37342] #32246
    • Add backtick escaping for two inline code samples in docs. [37338] #32246
    • Standardize on using :: for Class::method() references in WP_Customize_Control inline docs. [37337] #32246
    • Document the @return value of wp_add_trashed_suffix_to_post_name_for_post(). [37334] #36728

    Links

    • Rename the $link_id parameter in get_link_to_edit() to $link to better reflect that it can accept a link ID or object. [37353] #36736
    • Clarify documentation for the $link_id parameter to mention that it accepts either an integer or object. [37351] #36736

    Plugins

    • In plugin_basename() normalize the file path before unresolving symlinks. [37332] #29154
    • In uninstall_plugin() pass the plugin file to wp_register_plugin_realpath(). [37331] #36709

    Posts

    • Allow get_page_uri() to be called without a $page argument. [37345] #26284

    Query

    Tests

    • Ensure that image sizes are indeed removed when errors are raised before assertions in Tests_Media. [37328] #36588
    • Ensure that the GD absrtraction is used for GD unit tests for Images. [37327] #36588
    • Remove debug cruft left over from [34816]. [37344] #17078
    • Reduce unnecessary count in create_many() in multisite user tests. [37318] #36566

    Themes

    Users

    Widgets

    • Provide PHP 5.2 fallback for spl_object_hash() if disabled in logic for registering and unregistering pre-instantiated widgets. [37333] #28216
    • Allow WP_Widget subclass instances (objects) to be registered/unregistered in addition to WP_Widget subclass names (strings). [37329] #35990, #28216
    • When the Inactive Widgets section is hidden also hide the “Clear Inactive Widgets” button description text. [37323] #35592

    Props

    Thanks to @afercia, @andy, @boonebgorges, @celloexpressions, @chandrapatel, @DrewAPicture, @ericlewis, @flixos90, @flyingdr, @Frank-Klein, @jdgrimes, @jeremyfelt, @jsternberg, @kjbenk, @martinkrcho, @mdwheele, @michaelarestad, @netweb, @ocean90, @PeterRKnight, @pollett, @purcebr, @r-a-y, @rachelbaker, @SergeyBiryukov, @Shelob9, @tloureiro, @voldemortensen, @vortfu, @websupporter, @welcher, @westonruter, and @wonderboymusic for their contributions!

     
  • Weston Ruter 10:52 pm on May 4, 2016 Permalink |
    Tags: ,   

    Improving Setting Validation in the Customizer 

    In #34893 and the accompanying Customize Setting Validation feature plugin I’ve suggested improvements to the Customizer setting validation model. More can be read about the proposal in that ticket description and plugin readme, but the short of it is that settings in the Customizer generally undergo clean-up sanitization but lack a robust system for pass/fail validation. Here is a video demo depicting what I think validation should look like in the Customizer:

    Normally the Customizer just sanitizes values by attempting to coerce them and clean them up into something that can be safely used (e.g. stripping tags). As for validation, and while I believe this is relatively unusual to encounter, you can also do strict validation of a setting by blocking it from being saved: this is done by returning null from WP_Customize_Setting::sanitize() (often via  WP_Customize_Setting::$sanitize_callback). This is the behavior for setting the background_color: if the value is not a valid hex code, it will not save. The problem here is that there is no feedback to the user that the save was blocked. If user tries to enter “blue” as a color instead of a hex code, they will not get informed that this is invalid: it will be as if they never tried to save any change at all. Incidentally, this is also the case for widgets, where the WP_Widget::update() normally just sanitizes the instance arrays, though it can return false to block an update, with likewise no indication that the update was blocked.

    Additionally, in the current validation system, if you have made 10 changes in your Customizer session (transaction), but half of them get blocked from saving due to being invalid, then you have an only partially-saved Customizer state. This is a really bad experience for sites that make heavy use of the Customizer beyond just using it to tweak some colors.

    So to summarize the suggested improvements to validation in the customizer:

    1. All modified settings should be validated up-front before any of them are saved.
    2. If any setting is invalid, the Customizer save request should be rejected: a save thus becomes transactional with all the settings left dirty to try saving again.
    3. Validation error messages should be displayed to the user, prompting them to fix their mistake and try again.

    All of the above is implemented in the Customize Setting Validation feature plugin, and I’d love your feedback on it. One thing in particular I need feedback on is how specifically a setting should go about performing validation on the server. As noted above, the WP_Customize_Setting::sanitize() method can currently do both sanitization (returning a cleaned-up value) and validation (returning null to block saving if value is beyond repair). In the current feature plugin, I’m proposing:

    1. Continue to use the same sanitize methods/callbacks/filters.
    2. Introduce a new $strict param to indicate that validation is being performed.
    3. Allow these functions to fail validation by returning WP_Error instances with information about what failed in addition to null as they can today.

    How the changes would look for WP_Customize_Setting::sanitize():

    /**
     * Sanitize an input.
     *
     * @since 3.4.0
     * @since 4.6.0 Added `$strict` param to indicate validation is being performed.
     *
     * @param string|array $value  The value to sanitize.
     * @param bool         $strict Whether validation is being performed.
     * @return string|array|null|WP_Error Null or WP_Error if an input isn't valid, otherwise the sanitized value.
     */
    public function sanitize( $value, $strict = false )
    

    And for the customize_sanitize_{$id} filter:

    /**
     * Filter a Customize setting value.
     *
     * @since 3.4.0
     * @since 4.6.0 Added `$strict` param to indicate validation is being performed.
     *
     * @param mixed                $value  Value of the setting.
     * @param bool                 $strict Whether validation is being performed.
     * @param WP_Customize_Setting $this   WP_Customize_Setting instance.
     */
    return apply_filters( "customize_sanitize_{$this->id}", $value, $this, $strict );
    

    This should be nicely backwards-compatible, and allow for the sanitize/validate logic to be kept in the same function since they are very closely related. But that raises some questions:

    1. Should there be a separate WP_Customize_Setting::validate() method as well that by default calls WP_Customize_Setting::sanitize() with the $strict param set to true?
    2. Should validation only be applied when saving the setting while mere sanitization is required for previewing a value change?

    On the second point here, my thought is that it may often be the desire for looser cleanup sanitization to be applied to a value during data entry. Consider a blob of text displayed on the page which forbids the use of markup. As I do data entry I may like to see what I am entering being displayed in the preview with the tags stripped (normal behavior today) whereas when I try saving I can be alerted that saving the value with markup is not allowed. This would provide the user with feedback as to why the markup is mysteriously not appearing in the preview. (As for providing this validation feedback while the user entering data, this should be done via JS, or the validation state be determined by performing validation in an update-transaction request for transactions.)

    I’ll also note that REST API supports both sanitization and validation for request args, but these are handled in two separate callbacks. The way it works is that any registered sanitize_callbacks are applied first, followed by setting of default values, and then the validate_callbacks are run. See WP_REST_Server::dispatch(). This works fine, but I think we could have more flexibility if the sanitization and validation logic could be in the same method. Perhaps you want to run sanitization before validation (sanitize by stripping tags and validate to ensure the value is not empty), or be more strict by running validation before sanitization (reject any text that contains markup entirely).

    So, for you all who develop settings in the Customizer, what are your thoughts on how validation should behave, and how validation should relate to sanitization in the context of the Customizer? What do you think of the API changes I’ve proposed?

     

     
    • Aristeides Stathopoulos 12:14 am on May 5, 2016 Permalink | Log in to Reply

      I like it!

      I wanted to do something similar for a “dimension” control in the Kirki plugin and ended up doing it via JS (demo on https://youtu.be/2cM6WPKNdfs, JS validation function on https://github.com/aristath/kirki/blob/2.3.2/assets/js/functions/validate-css-value.js) but I like your approach a lot better than mine… and we definitely need it.

      Should there be a separate WP_Customize_Setting::validate() method as well that by default calls WP_Customize_Setting::sanitize() with the $strict param set to true?

      I believe that would be best, yes. validation and sanitization are different concepts, they should have different functions, even if internally they’re the same thing. It’s a simple thing to do, but will save us a lot of trouble down the road in support and explaining things to people. We should make it easier to understand, and “use the validate method to validate the function” is a lot better than “add a 2nd argument to the sanitization function function and set it to true if you want to validate instead of sanitize”.

      Should validation only be applied when saving the setting while mere sanitization is required for previewing a value change?

      Shouldn’t that be the other way around? Or is my migraine playing tricks on me?
      IMO we should have sanitization on save, and validation to allow/disallow saving, and also to trigger/skip triggering the preview refresh/postMessage.

      • Weston Ruter 12:50 am on May 5, 2016 Permalink | Log in to Reply

        @aristath:

        Shouldn’t that be the other way around? Or is my migraine playing tricks on me?
        IMO we should have sanitization on save, and validation to allow/disallow saving, and also to trigger/skip triggering the preview refresh/postMessage.

        Maybe what I wrote isn’t very clear. I meant to say that in both the case of previewing a setting change and saving a setting, the sanitization should be applied. I was suggesting that the validation (strict sanitization) would be an additional check that is done only on save when persisting the value to the database.

        But it sounds like you’re saying that validation should be done every time in addition to sanitization. Is that right? It would be a challenge to block previewing changes to such values since at the moment the sanitize callback is normally only run after a refresh has been requested, and for postMessage previews there is normally no server-side communication at all, so PHP sanitization/validation logic would be involved, and it would be up to JS to implement the setting.validate() method to catch those issues.

        For settings with the refresh transport, there would be a possibility to use PHP validation to block previewing of invalid settings only if transactions is implemented. How I think transactions will work is every time you make a change to a setting, the Customizer would first do an Ajax request to push the new changed setting into the current transaction post. Once the value has been saved into the post and the update-transaction Ajax request returns, then a refresh can be done so that the preview can re-render with the new value stored in the transaction. It’s here with the update-transaction request that we could call the PHP setting validation function and the response could include any failure information, and then prevent the preview from refreshing. Does that make sense?

    • Aristeides Stathopoulos 1:25 am on May 5, 2016 Permalink | Log in to Reply

      it sounds like you’re saying that validation should be done every time in addition to sanitization. Is that right?

      Correct. Just a thought though, it has its pros and cons…

      As for blocking refreshes etc, I agree that it’s going to be hard via PHP, so perhaps we should consider implementing something in the JS side of the Customizer API too?

      We can add a validate method on the PHP side, and we could also just extend the controls in JS and add a validation method there too. This way it’s up to the developer how they’re going to implement it.
      If they need realtime validation and blocking refreshes for example when a value is invalid as I did, then using the JS method makes more sense.
      If they need something simpler or just don’t need to block the preview refresh they can use the PHP method.

      The transactions concept makes sense… but that would only work on settings with `transport` set to `refresh`, right? From what I’ve seen people are slowly trying to get away from that and are trying to implement `postMessage` more and more.
      So again, we’d need a way to make it work no matter what they’re using as `transport`.

      • Weston Ruter 3:40 am on May 5, 2016 Permalink | Log in to Reply

        @aristath:

        As for blocking refreshes etc, I agree that it’s going to be hard via PHP, so perhaps we should consider implementing something in the JS side of the Customizer API too?

        This can be done today, although I don’t think I’ve ever seen it done in the wild. Consider:

        
        wp.customize('blogname').validate = function( value ) {
            if ( /<\/?\w/.test( value ) ) {
                console.warn( 'Markup not allowed in site title.' ); 
                value = null; 
            } 
            return value;
        };

        This has the effect of blocking the user from being able to supply an illegal value. The console.warn() could be replaced with a call to update some validation message instead, or we could improve core to allow these validate functions to even return an Error object for parity with the PHP pattern I showed above.

        The transactions concept makes sense… but that would only work on settings with `transport` set to `refresh`, right? From what I’ve seen people are slowly trying to get away from that and are trying to implement `postMessage` more and more.

        To prevent invalid setting values from being sent to the preview, yes. That is unless they implement a JS-based validate as well, as I showed with the blogname above.

        I’m thinking more now that validation should be done with sanitization in all cases now, instead of just when saving.

      • Weston Ruter 7:22 am on May 5, 2016 Permalink | Log in to Reply

        @aristath: Please give a try to the latest version of Customize Settings Validation (on GitHub) and try it with the JS Widgets. Then add a Text widget and you should see some helpful validation messages appear when you try entering markup into the title field. Here’s the corresponding logic that handles that: https://github.com/xwp/wp-js-widgets/blob/dce6d507ebade7b645220a9c843bb2b02198ef9a/js/widgets/customize-widget-text.js#L90-L98

        • Aristeides Stathopoulos 10:31 am on May 5, 2016 Permalink | Log in to Reply

          Please give a try to the latest version of Customize Settings Validation (on GitHub) and try it with the JS Widgets.

          Pretty sleek and simple to implement!

          To prevent invalid setting values from being sent to the preview, yes. That is unless they implement a JS-based validate as well, as I showed with the blogname above.

          If we implement PHP-based validation in-core, JS-based validation **should** be part of core as well.

          I’m thinking more now that validation should be done with sanitization in all cases now, instead of just when saving.

          Agreed 100%

    • NateWr 9:24 am on May 5, 2016 Permalink | Log in to Reply

      Sounds great.

      So looking at the code, it looks like each control needs to have a dummy element inside it’s template with the class `.customize-setting-validation-message`:

      https://github.com/xwp/wp-customize-setting-validation/blob/master/js/customize-setting-validation.js#L104

      Do you have any thoughts on how this could be used for controls that manage multiple settings?

      • Weston Ruter 3:31 pm on May 5, 2016 Permalink | Log in to Reply

        @natewr

        Yes, each control should have a container for where the validation message should be displayed. With the discussion of having a global notification area (#35210) and the need to be able to display control-level notifications (#29932), I think the .customize-setting-validation-message element should be replaced with a .customize-control-notifications and have parity with whatever is developed for the global notifications. In the same vein, the JS collection containing the notifications should be like control.notifications instead of control.validationMessages since there wouldn’t necessarily be a one-to-one correspondence between the notifications and the settings associated with the control.

        Also, the actual logic for displaying the notifications based on changes to the control.notifications collection should be the responsibility of the control and be able to be overridden by control subclasses.

        So all of that to say, I think that the API should support multiple notifications being displayed, and this could tie in well with controls that are associated with multiple settings.

    • Ahmad Awais 10:32 am on May 5, 2016 Permalink | Log in to Reply

      Perfect!

      I just tested Customize Settings Validations with JS Widgets.

      Is there something we need to do? I just Cloned the master branch and it ain’t validating anything like in the video above. Thought it’d be plug n play.

      About the feedback, here it goes

      1- Should there be a separate WP_Customize_Setting::validate() method as well that by default calls WP_Customize_Setting::sanitize() with the $strict param set to true?

      Yes, if it will help us build custom validations (custom stuff) routine and in a neat way in near future.

      Should validation only be applied when saving the setting while mere sanitization is required for previewing a value change?

      I think that would be more efficient. Inclined to agree here.

      • Weston Ruter 3:21 pm on May 5, 2016 Permalink | Log in to Reply

        @mrahmadawais

        Is there something we need to do? I just Cloned the master branch and it ain’t validating anything like in the video above. Thought it’d be plug n play.

        Currently only the Text widget has been implemented, and the WP REST API plugin is a dependency for JS Widgets. Another demo plugin you can use to test with is “Customize Validate Entitled Settings.

        Yes, if it will help us build custom validations (custom stuff) routine and in a neat way in near future.

        But why not just return the WP_Error instances from the existing WP_Customize_Setting::sanitize() functions and look at this value to see if valid? I’m trying to see why there would need to be a different implementation of WP_Customize_Setting::validate() that would do anything besides return is_wp_error( $this->sanitize( $value ) );. Should the behavior be any different?

        I think that would be more efficient. Inclined to agree here.

        It wouldn’t be any more efficient actually. It’s whether or not you should be able to preview invalid-yet-sanitized values. The more I think about it, this doesn’t really make sense. I’m sure most devs would expect invalid values to always be rejected, and for validation and sanitization to always be done together.

        • Ahmad Awais 5:45 pm on May 5, 2016 Permalink | Log in to Reply

          It wouldn’t be any more efficient actually. It’s whether or not you should be able to preview invalid-yet-sanitized values. The more I think about it, this doesn’t really make sense. I’m sure most devs would expect invalid values to always be rejected, and for validation and sanitization to always be done together.

          OK! I see where you are coming from but I think if keep both of these functions separate it would make the code look more neat and modular. Maybe having a parameter set to true or false that could lead to `return is_wp_error( $this->sanitize( $value ) );` or `return $this->sanitize( $value );`.

          That said, I agree, keep validation and sanitization together would make more sense, sine anyone who is validating an input would want similar sanitization for the setting in question.

          • Weston Ruter 6:09 pm on May 5, 2016 Permalink | Log in to Reply

            @mrahmadawais My thinking was that the validate method would always return bool or WP_Error, similar to how WP_REST_Request::has_valid_params() looks for validate_callbacks (like rest_validate_request_arg()) to that return such pass/fail values. In the same way, a WP_Customize_Setting::validate() could be a final method that is defined as:

            $sanitized = $this->sanitize( $value );
            return ! (
                is_null( $sanitized ) 
                || is_wp_error( $sanitized ) 
            );

            The null check would be for legacy sanitizers that aren’t updated to return more descriptive WP_Error instances.

    • Aristeides Stathopoulos 1:49 pm on May 5, 2016 Permalink | Log in to Reply

      @mrahmadawais you need to clone the git repository recursively, then npm install && grunt

      • Weston Ruter 3:32 pm on May 5, 2016 Permalink | Log in to Reply

        @aristath Good point, however this should only be required if SCRIPT_DEBUG is false.

        • Ahmad Awais 5:47 pm on May 5, 2016 Permalink | Log in to Reply

          For some reason, I am still not able to test it. I had cloned the repo recursively but haven’t done the npm pacakge installs and grunt. I did that, but nothing changed. Then I checked there is a composer file so I installed the composer dependency as well. Still no luck.

          What could I be doing wrong?

    • Weston Ruter 6:09 am on May 11, 2016 Permalink | Log in to Reply

      I just pushed out 0.1.1 which fixed a couple bugs: https://wordpress.org/plugins/customize-setting-validation/

    • Weston Ruter 7:58 am on May 15, 2016 Permalink | Log in to Reply

      @mrahmadawais @aristath @natewr Please watch #34893 and see my latest comment there: https://core.trac.wordpress.org/ticket/34893#comment:24

      After a few iterations, I came to the conclusion that there needs to be separate validate methods/filters/callbacks for back-compat with plugins that may be filtering or calling the sanitize ones and expecting that these always return the value or null, not a WP_Error instance.

      I’ve updated the sample extension plugin to make use of this patch: https://gist.github.com/westonruter/1016332b18ee7946dec3

      Please de-activate the Customize Setting Validation plugin.

    • Weston Ruter 5:28 pm on May 16, 2016 Permalink | Log in to Reply

      @mrahmadawais @aristath @natewr cab you give any final feedback of the latest patch on the ticket? I want to commit today so we can move on to transactions.

  • Hugo Baeta 9:48 pm on May 4, 2016 Permalink |
    Tags: , cross-posting, design chat   

    x-post from make/design:

    Weekly Design Chat revival + call for topics

     
  • Konstantin Obenland 7:43 pm on May 4, 2016 Permalink |
    Tags: , ,   

    Shiny Updates Chat Summary 5/3/16 

    This is a summary of the shiny updates chat from May 3. (Slack log)

    Attendees: @ocean90, @swissspidy, @ethitter, @adamsilverstein, @mapk, @obenland

    Topics:

    • Accomplished work since last week
    • Remaining work
      @obenland will work on the Shiny Updates landing page for feature projects and fix a bug that were discovered during the most recent user test. @mapk offered to conduct another user test focused on the new interactions with some learnings from the previous one. Goal is to identify any hiccups or behaviors that are unexpected to users. @swissspidy wants to continue working on update-core.php and see if we can get it into shape for 4.6.
    • Most recent user tests
      We briefly talked about the most recent user test (linked above). It showed a variety of bugs that @obenland initially assumed were based on being in subviews of the plugin list. After the meeting it turned out that the test site ran an outdated version of WordPress that didn’t include some of the date that Shiny Update needs to work. It was a good demonstration of progressive enhancement here though, as all actions still worked, they just weren’t shiny. More user tests to come, to make sure that users agree with the new interactions.
    • Schedule for 4.6 inclusion
      Next up is writing the merge proposal that was requested to be published around 6/1, definitely before 6/8.

    Next meeting is on Tuesday May 10, 19:00 UTC.

     
    • Scott Fennell 7:55 pm on May 4, 2016 Permalink | Log in to Reply

      Hello! I have a question about the shiny updates work — and thanks for the work, by the way.

      I have written a plugin for my own personal use (not a .org plugin) which allows me to use the core update class for updating plugins/themes that are hosted in a private bitbucket account. It works by flitering the plugin/theme data at `pre_set_site_transient_update_plugins` and `pre_set_site_transient_update_themes`.

      I was wondering if the shiny updates work could jeopardize either of those filters. Would you be able to comment on this concern?

      Aside: My plugin is pretty similar to a more well-known project called GitHub Updater.

      • Konstantin Obenland 8:03 pm on May 4, 2016 Permalink | Log in to Reply

        Hi Scott, thanks for your feedback!

        Shiny Updates doesn’t use these filters, it’s more focused on the UI of installing/updating/deleting plugins and themes. I’d encourage you though to download and activate the plugin to test how it interacts with your plugin just to make sure (and maybe let us know if you encounter any bugs or weird behavior).

    • Andy Fragen 8:03 pm on May 4, 2016 Permalink | Log in to Reply

      Hey Scott! Aside from some styling changes that will likely need to be made, the functionality of GHU and Shiny Updates work great together.

c
compose new post
j
next post/next comment
k
previous post/previous comment
r
reply
e
edit
o
show/hide comments
t
go to top
l
go to login
h
show/hide help
shift + esc
cancel
Skip to toolbar