Revamp Press This

[michael-arestad]

This will be used as the master tracking ticket for the ​Press This Revamp feature plugin, which has been approved for merge in the 4.2 release.

• GitHub Repo: ​https://github.com/MichaelArestad/Press-This
• Plugin Page: ​https://wordpress.org/plugins/press-this/

[helen]

#21551 was marked as a duplicate.

[kraftbj]

Tickets that can be modified once this is merged in:

Fixed:
#16922 - PT needs a class for the source attribution
#16935 - Selected text wrapped in blockquote
#18937 - Style PT header like admin bar
#27529 - Update media icons

Need Reporter Feedback:
#16940 - Does the new bookmarklet cause problems for mod_security?

IMO wontfix:
#7605 - Ability to edit slug via PT pop-up
#14650 - PT to use post-new.php

Should get a response updated with where PT now stands and the future roadmap:
#26681 - Grab HTML with selected text
#30803 - Selected image not used as featured image
#28873 - Improve keyboard accessibility of adding PT JS
#26794 - Remove PT code from Settings->Writing

Remaining tickets, as of now:
#14164 - do_meta_boxes on PT page
#12017 - Add site selector for multisite to allow one bookmarklet to be used on any blog user has an acceptable role on.

[azaozz]

In 31373.patch: the plugin converted to a core patch (160KB...). Still some things to iron out/decide.

[stephdau]

@drewapicture: your version of the patch is at least missing src/wp-admin/js/bookmarklet.js, which leads to the following when someone drags the bookmarklet to their bookmark bar:

https://src.wordpress-develop.dev/wp-admin/%3Cbr%20/%3E%3Cb%3EWarning%3C/b%3E:%20%20file_get_contents(
/srv/www/wordpress-develop/src/wp-admin/js/bookmarklet.js):%20failed%20to%20open%20stream:%20No%20such
%20file%20or%20directory%20in%20%3Cb%3E/srv/www/wordpress-develop/src/wp-includes/link-template.php%3C
/b%3E%20on%20line%20%3Cb%3E2640%3C/b%3E%3Cbr%20/%3Ejavascript:

[stephdau]

@azaozz: your version works great!

Couple of things to think about:

• will have to tweak the logic that leads to the "you should upgrade your bookmarklet" notice, as it seems to show at al times now, despite having the right version: ​https://cloudup.com/cixoVM9Ar2K
• the tools.php screen could now use a design pass altogether to make its sections more consistent: ​https://cloudup.com/cGePU4Ai5eA

[stephdau]

Upgrade notice: the culprit is !== and different types between data.v and data._version.
Using parseInt() when comparing them makes the problem go away.

wp-admin/js/press-this.php -> L640 (in 31373.patch​):

// Prompt user to upgrade their bookmarklet if there is a version mismatch.
if ( data.v && data._version && parseInt( data.v ) !==  parseInt( data._version ) ) {

[DrewAPicture]

Docs: 1st pass audit

[DrewAPicture]

Docs: 2nd pass audit + file fixes

[azaozz]

In 31373.4.patch: some css fixes/enhancements for the Tools screen by @michael-arestad.

[Michael Arestad]

I've already found a few bugs. I created tickets against trunk in the Press This component. The titles are prefixed with "Press This:".

Tab order bug in Safari
Black border in IE
Convert categories list to li because of major display bugs

More testing in a bit.

[azaozz]

In 31534:

PressThis v2, first run. Props michael-arestad, stephdau, marcelomazza, DrewAPicture, iseulde, afercia, kraftbj, rachelbaker, AramZS, dd32. See #31373.

[azaozz]

In 31535:

PressThis:

• Hard-code the minified bookmarklet js. Adding the non-minified bookmarklet to the browser bookmarks bar may have unexpected effect.
• Fix type juggling when checking the bookmarklet version.

Props stephdau, see #31373.

[azaozz]

In 31537:

Press This: JSON encode the URL before appending it to the bookmarklet. See #31373.

[kraftbj]

Noting that I closed (as worksforme since best I could do) #16922, #16935, #18937, #26794, #27529 per the above list.

[stephdau]

W00t w00t.

[azaozz]

In 31545:

PressThis: go back to loading the minified bookmarklet content with file_get_contents(). Add Grunt task to minify bookmarklet.js on precommit and update it in /src. See #31373.

[azaozz]

In 31547:

Press This: add press-this.css to the list of stylesheets that are minified. Bump wp_version. See #31373.

[Michael Arestad]

Noting 31455 here. Should fix FF scrolling issues.

[Michael Arestad]

Added patch for 31456 (which is an awesome number) that just adds a class to make alert styles work properly.

[Michael Arestad]

Some feature request tickets:

31457: Close sidebar when cursor clicks out of sidebar or tab focus leaves sidebar
31458: Add Preview functionality

[Michael Arestad]

Noting accessibility patch 31449 by @afercia

[kraftbj]

Noting updated upgrade bookmarklet patch in #31461.

[kraftbj]

Open question on styling of tools.php with the "new" style of the Press This area in #31462

[ocean90]

In 31571:

Press This: Use boolean value instead of return_true(). Add missing hook docs.

see #31373.

[ocean90]

In 31572:

Press This: Add press-this to list of RTL styles.

see #31373.

[ocean90]

In 31588:

Press This: Backwards compatibility enhancements.

• Add missing actions for printing styles/scripts.
• Since $hook_suffix is null, hardcode press-this.php.
• Restore body classes, add filter.
• Use boolean value instead of __return_false().
• Use wp_json_encode().
• Update docs for filters in script-loader.php.
• Make <a href="%1$s">%2$s</a> not translatable.

see #31373.

[Michael Arestad]

Noting removal of class on suggested blockquote and source: 31493

[azaozz]

Replying to ocean90:

• Add missing actions for printing styles/scripts.

Not sure this is a good idea. Many plugins do many things in the admin. Actually was thinking we shouldn't fire any of the default admin actions but only Press This specific actions.

• Since $hook_suffix is null, hardcode press-this.php.

$hook_suffix is set in wp-admin/admin.php loaded from press-this.php: https://core.trac.wordpress.org/browser/trunk/src/wp-admin/press-this.php#L12

If it is not set in some circumstances, we still need to set it globally as many plugins use it to determine when to output scripts/styles and some are probably looking at the global.

• Restore body classes, add filter.

Again, not sure that is "wise", at least not for all the body classes :) For example currently Press This doesn't support admin colors (the PT css is completely separate from the rest of wp-admin.css). Having a color class may trigger something we don't want. If a plugin adds something that triggers on a particular body class, and expects wp-admin.css to be loaded, it will fail and might affect Press This.

Was thinking we could/should move the HTML generation inside press-this.php, so it is inline with the rest of the admin "entry points" (the way it used to be). That means it will run directly in global scope, like the rest of wp-admin.

[azaozz]

Actually $hook_suffix should never be null, admin.php should always be loaded (in global scope) before calling WP_Press_This::html().

[azaozz]

In 31589:

PressThis:

• Remove unneeded passing of post formats strings to JS.
• Set the currently selected post format name with jQuery.

See #31373.

[kraftbj]

Super minor inline docs typo (s/fr/for) in class-wp-press-this.php

[azaozz]

In 31595:

PressThis:

• Simplify getSuggestedContent() and helpers. No need to override the global data.
• Replace the press_this_source_string and press_this_source_link filters with press_this_suggested_html that allows filtering of the link and the wrapper HTML tags.

See #31373.

[azaozz]

In 31596:

PressThis:

• Replace all %1$s and %2$s in suggestedHTML in case plugins repeat them.
• Fix docs typo, props kraftbj.

See #31373.

[Michael Arestad]

Updated buttons here: 31498

[azaozz]

In 31597:

PressThis: make sure buttons.css is loaded before press-this.css. Use (int) $post_ID instead of $post->ID.
See #31373.

[kraftbj]

Forgot to note here #31494 (make the suggested HTML filterable) which @azaozz did today in [31595] and so is safe to mark as fixed.

[azaozz]

In 31609:

PressThis:

• Improve handling of the data, both from the bookmarklet and from server-side parsing.
• Standardize on processing the data in PHP and remove duplicate code from JS.
• Improve the bookmarklet code and remove pre-filtering of the data.

Part props stephdau, see #31373.

[stephdau]

Cleans ip the use of unnecessary ( vartest && vartest.length ) in JS in press-this.php, as Ox did in bookmarklet.

[stephdau]

Same as length-less.diff, but with ticket name in file (can't delete/overwrite other one, sorry)

[stephdau]

Fixes a bug where we test for empty( $key ), but $key can legitimately be 0 for the 1st key of an indexed array (_embed, _img).

[stephdau]

Adds DailyMotion embed detection in non-DailyMotion pages (as we do for others). Will discover DM embeds on any site/page.

[azaozz]

In 31613:

PressThis: remove the extra .length tests for strings from press-this.js.
Props stephdau, see #31373.

[stephdau]

Added 2 more small patches:

• 31373-empty-fix.diff​: Tweaks an empty() test.
• 31373-dailymotion.diff​: Better DailyMotion embed support.

The equivalent of the length-less patches was committed in r31613.

[stephdau]

Adds support for Youtube playlists (supported by editor as WP embed), on youtube.com or when iframe embed is found on target page.

[stephdau]

Added 31373-ytplaylists.diff​ to add support for Youtube playlists.

[azaozz]

In 31635:

PressThis:

• Fix error when checking for empty array keys.
• Add better DailyMotion embed support.

Props stephdau, see #31373.

[azaozz]

In 31636:

PressThis: fix toggling of aria-expanded attribute.
Props afercia, see #31373.

[azaozz]

In 31637:

PressThis: when server-side parsing, filter small images by the width and height attributes if set.
See #31373.

[tyxla]

Related: #31558.

In this ticket I've proposed a fix for an issue with duplicate initialization of tagBox in wp-admin/js/tags-box.js.

[azaozz]

In 31373.8.patch:

• Move filtering of the data from JS to PHP. No need to send all data to JS and filter it there, can send only useful data.
• Clean up the links passed from the bookmarklet. We are using only canonical, if found.
• Load the suggested title and post content from PHP, no need to send to JS first.

[azaozz]

In 31680:

PressThis: fix styling of wpLink.
Props Michael-Arestad, see #31373.

[azaozz]

In 31373.9.patch: fixed few typos and inconsistencies, added bookmarklet.min.js so it can be tested easier (no need to run grunt precommit before updating the bookmarklet).

[stephdau]

Same as 31373.9.patch​, but with WP_Press_This::get_embeds() fixed (needed to use $data['_embed'], not $data['_embeds']).

[stephdau]

Replying to azaozz:

In 31373.9.patch: fixed few typos and inconsistencies, added bookmarklet.min.js so it can be tested easier (no need to run grunt precommit before updating the bookmarklet).

See 31373.10.patch​ for a small fix to WP_Press_This::get_embeds().

[stephdau]

Alternatively, and maybe for the better, we could also standardize on the use of _embeds and _images for those keys everywhere (bookmarklet, app JS/PHP), instead of now _img, _embed, _images, and _embeds. We uglify the bookmarklet now, so we no longer need to save on characters there. :)

[azaozz]

In 31693:

PressThis:

• Filter and select the content on the PHP side. Then pass only the needed data to JS.
• Add the suggested post title and contend directly to the HTML.
• Standardise the data type names.
• Some cleanup/reduction of the code in the bookmarklet.

See #31373.

[azaozz]

Replying to stephdau:

...we could also standardize on the use of _embeds and _images for those keys everywhere (bookmarklet, app JS/PHP), instead of now _img, _embed.

Right, good idea. Done in the commit. Also, thanks for the typo fixes :)

[stephdau]

We're having a problem with UTF-8 URLs: the characters are being stripped by sanitize_text_field() in WP_Press_This::_limit_string() right after the URL was html entity decoded, with UTF-8 compatibility, because WP_Press_This::_limit_url() uses it.

So the URL ​http://tekartist.org/2015/03/10/%e2%99%ab-phantogram-when-im-small-live-at-kexp/ becomes ​http://tekartist.org/2015/03/10/-phantogram-when-im-small-live-at-kexp/ after sanitization, which leads to a 404. The bookmarklet appears to work on that URL because it passes some valid meta data and does not fetch the source, but the attribution link is bad. If you use the same URL in the "Scan URL" form, the fetch fails altogether, for the same reason.

Should we consider not running sanitize_text_field() on strings that look like URL?

Index: src/wp-admin/includes/class-wp-press-this.php
===================================================================
--- src/wp-admin/includes/class-wp-press-this.php	(revision 31694)
+++ src/wp-admin/includes/class-wp-press-this.php	(working copy)
@@ -330,8 +330,10 @@
 				$return = $value;
 			}
 
-			$return = html_entity_decode( $return, ENT_QUOTES, 'UTF-8' );
-			$return = sanitize_text_field( trim( $return ) );
+			$return = trim( html_entity_decode( $return, ENT_QUOTES, 'UTF-8' ) );
+			if ( ! preg_match( '/^https?:/', $return ) ) {
+				$return = sanitize_text_field( $return );
+			}
 		}
 
 		return $return;

Or simply stop WP_Press_This::_limit_url() from using WP_Press_This::_limit_string(), since it starts with an is_string() test?

Index: src/wp-admin/includes/class-wp-press-this.php
===================================================================
--- src/wp-admin/includes/class-wp-press-this.php	(revision 31694)
+++ src/wp-admin/includes/class-wp-press-this.php	(working copy)
@@ -342,8 +342,6 @@
 			return '';
 		}
 
-		$url = $this->_limit_string( $url );
-
 		// HTTP 1.1 allows 8000 chars but the "de-facto" standard supported in all current browsers is 2048.
 		if ( mb_strlen( $url ) > 2048 ) {
 			return ''; // Return empty rather than a trunacted/invalid URL

[stephdau]

Better matching for WordPress.co stats gifs.

[stephdau]

Better Youtube URL matching, and better embed docs.

[stephdau]

Also added 2 small patches:

• 31373-wpcom-stats.diff​: Better matching for WordPress.com stats gifs.
• 31373-better-embeds-and-docs.diff​: Better Youtube URL matching, and better embed docs.

[azaozz]

In 31737:

PressThis: update _limit_url(), use esc_url_raw(). Fixes checking of urlencoded strings.
See #31373.

[azaozz]

In 31739:

PressThis: add inline comments and some cleanup for the images and embeds regex.
Props stephdau. Fixes #31373.

[azaozz]

In 31774:

PressThis: in Grunt, minify the bookmarklet before copying the files to /build.
See #31373.

[azaozz]

In 31778:

PressThis: increase the number of meta tags we check in the bookmarklet to 200. Sometimes there are empty meta tag nodes (no attributes) in some browsers.
See #31373.

[Stagger Lee]

Dont know where to ask. Sorry if it is wrong place.

Did you reconsider making it easy to change CSS style of Press This editor container ? (press-this-editor.css)

Wordpress is about web editors beginners. And I personally find it very scary that they are faced with one screen where they have to insert something publicly visible on their websites, but CSS style of text formatting is light years away from style of their websites.

[azaozz]

Replying to Stagger Lee:

Did you reconsider making it easy to change CSS style of Press This editor...

Not sure what you mean by "make it easy". It is as easy as using a filter to pass another stylesheet URL. You can use one of the specific Press This actions and add another URL with mce_css. Something like this should work fine:

add_action( 'admin_head-press-this.php', 'my_pt_styles' );
function my_pt_styles() {
  add_filter( 'mce_css', 'my_press_this_editor_css' );
}

function my_press_this_editor_css( $css ) {
  if ( ! empty( $css ) {
    $css .= ',';
  }

  $css .= plugins_url( 'my-subdir/my-pt-style.css', __file__ );

  return $css;
}

[Stagger Lee]

Thank you. Will save snippet and use it. Best to link to editor style Advanced TinyMce plugin use, and both are done at once. Change path to theme root.

One more thing. Breaks and paragraphs in text. Can you bring it back and respect as it is on remote website ?
One question here is to be asked. Is it more harm done for look of post by few small problems on remote website, or it is more harm done by putting all text in one paragraph without any spacing. Think answer is oblivious, but it is just my personal opinion.

[michaelarestad]

One more thing. Breaks and paragraphs in text. Can you bring it back and respect as it is on remote website ?

I'm not sure I understand. Can you give an example?

[Stagger Lee]

Let us say you want to copy one editorial from Guardian newspapper. When all in Press This is finished all text ends in just one paragraph. No breaks, no lines, no other paragraphs as on original remote website (post).

If you use TinyMce Advanced plugin and button in editor "Ad as text" or what it is called dont remember now, before pasting text. After text pasting all breaks and paragraphs from remote website are preserved.

Mathematically speaking it takes much, much, much less time to fix formatting when just copy/paste text from remote website, than when dealing with Press This.

[DrewAPicture]

Replying to Stagger Lee:

Let us say you want to copy one editorial from Guardian newspapper. When all in Press This is finished all text ends in just one paragraph. No breaks, no lines, no other paragraphs as on original remote website (post).

@michaelarestad FWIW, I've seen this too. The formatting is correct in the Press This editor but once you make it over to the standard editor, all the formatting tags are in place but it's a giant block of text and markup with no line breaks or indentation. Haven't yet had a chance to track down where it's happening (it doesn't always).

[michaelarestad]

Oh! Right! Here's an open ticket for it. Would love someone to take a look. #33712

[Stagger Lee]

Talk to @azaozz, developer of the TinyMce Advanced plugin. His editor button "Paste as Text" makes wonder, all the time.