Make WordPress Core

Opened 13 years ago

Closed 13 years ago

#18626 closed enhancement (fixed)

Update user_can_richedit to allow TinyMCE with iOS5/contentEditable support

Reported by: markoheijnen's profile markoheijnen Owned by: azaozz's profile azaozz
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.3
Component: Editor Keywords:
Focuses: Cc:

Description

At this moment I updated my TinyMCE to 3.4.5 so it got contentEditable support for iOS5.
This is not enough because you need to change user_can_richedit(). Also after that the behavior of the editor needs to be changed.
Because now when you enter the fields get larger. It's most likely you want to have the same height.

Related tickets: #17144 && #18198

Attachments (2)

18626.patch (782 bytes) - added by SergeyBiryukov 13 years ago.
current_web_browser.patch (5.2 KB) - added by azaozz 13 years ago.

Download all attachments as: .zip

Change History (18)

#1 @markoheijnen
13 years ago

  • Cc marko@… added

Code sample of user_can_richedit that I use now.

function user_can_richedit() {
	global $wp_rich_edit, $pagenow, $is_iphone;

	if ( !isset( $wp_rich_edit) ) {
		if ( get_user_option( 'rich_editing' ) == 'true' &&
			( ( preg_match( '!AppleWebKit/(\d+)!', $_SERVER['HTTP_USER_AGENT'], $match ) && intval($match[1]) >= 420 ) ||
				!preg_match( '!opera[ /][2-8]|konqueror|safari!i', $_SERVER['HTTP_USER_AGENT'] ) ) &&
			!( $is_iphone && intval($match[1]) < 534 )
		) {
			$wp_rich_edit = true;
		} else {
			$wp_rich_edit = false;
		}
	}

	return apply_filters('user_can_richedit', $wp_rich_edit);
}

#2 @ocean90
13 years ago

  • Milestone changed from Awaiting Review to 3.3
  • Summary changed from Update TineMCE with iOS5/contentEditable support to Update user_can_richedit to allow TinyMCE with iOS5/contentEditable support
  • Type changed from feature request to enhancement

Related: #18107

#3 @technosailor
13 years ago

Personally, I'm not a fan of putting core code in to address one platform (iOS). Can we write this patch in such a way that ensures Android also has contentEditable?

While Android 2.3 does not have this support, 3.0 Honeycomb does.

http://caniuse.com/contenteditable

#4 @markoheijnen
13 years ago

  • Cc marko@… removed

You right about that. We can check with javascript for support but you can get false positives with it like on iOS4 and I believe also on Android 2.3.
Since some browsers have some kind of support for contentEditable but it isn't usefull for TineMCE.

#5 @markoheijnen
13 years ago

  • Cc marko@… added

#6 follow-up: @azaozz
13 years ago

Thinking that for now we still need to 'hand pick' the mobile browsers with full contentEditable support: iOS 5, Android 3, etc. Also $is_android is in order too imho.

Last edited 13 years ago by azaozz (previous) (diff)

#7 in reply to: ↑ 6 @technosailor
13 years ago

Replying to azaozz:

Thinking that for now we still need to 'hand pick' the mobile browsers with full contentEditable support: iOS 5, Android 3, etc. Also $is_android is in order too imho.

Would it break backcompat to have $is_ios (masked to $is_iphone) and $is_android return false or the version number (which is also true)?

That way appropriate targetting of platform can be done in core and by devs.

#8 @nacin
13 years ago

These globals all kind of suck. It'd be nice to bring them into a singleton or function rather than spawning new ones. Maybe even something that someone could extend and offer better detection.

Further, we should stop with device detection, and get into feature detection. Specifically, two features we need to know about for smart mobile phones -- are they touch (which we need to know for responsive stuff), and do they support TinyMCE. If we do is_iOS and is_android, we leave out WP7, which we really shouldn't.

#9 @markoheijnen
13 years ago

All the browser detection in /wp-includes/vars.php is indeed little bit to much.

Writing functions or singleton with all the detection is probably the way to go. Not sure what the way to go is since PHP is our definition and with javascript the browser gives us the information. We only need to check if the browser doesn't lie about the functionality like with contentEditable support

#10 @azaozz
13 years ago

Yes, the browser detection in /wp-includes/vars.php seem to be older than WP (before 12/11/2003) :)

However we still need some sort of hand picking browser name and version by the header strings especially for all the mobile browsers. Feature detection is fine but we don't need to output the whole TinyMCE when we can find out in PHP that it's not supported.

That saves quite a bit of bandwidth which is still pretty expensive for mobile devices in some regions (this includes tablets too).

Last edited 13 years ago by azaozz (previous) (diff)

#11 @technosailor
13 years ago

Yeah but... just like I don't like adding core code for a single platform, I don't like to add core code for a single use case. We shouldn't be trying to write this code only to determine if the browser can load TinyMCE. :)

#12 @azaozz
13 years ago

Well, we can't get rid of the globals for back-compat reasons, wondering how we can mark them as deprecated... Having the code in a singleton would work, although if plugins want to refine the detection they only need to reset the affected global, no need to extend a PHP class.

Perhaps we can do this:

  • leave the current globals as is
  • make a new one called current_web_browser that will be an array of supported features in the browser, like
    name => 'whatever',
    version => '###',
    contentEditable => false,
    mobile => true
    

etc.

Then we can detect and/or hand code all the mobile browsers without polluting the global scope further.

Last edited 13 years ago by azaozz (previous) (diff)

#13 @markoheijnen
13 years ago

Sounds great. Current code can be implemented within the new global.

Not sure if plugins use the globals. I do know the core use them wrong like checking if is_iphone to do stuff what should be mobile global.
That is something for another ticket.

#14 @GaryJ
13 years ago

  • Cc gary@… added

#15 @azaozz
13 years ago

First attempt for more useful $current_web_browser global. Will need more granular smart phone detection (when we start to support most/all of them).

#16 @azaozz
13 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In [18963]:

Update user_can_richedit to allow TinyMCE in iOS5, props markoheijnen, fixes #18626

Note: See TracTickets for help on using tickets.