Page MenuHomePhabricator

VisualEditor should not try to initialise if no ContentEditable support
Closed, ResolvedPublic

Description

Example: Android 2.x browser (tested Kindle Fire), iOS < 5 browser.

The editor initializes, but since there's no ContentEditable support you can't actually type anything. In iOS 4.3.2 simulator initialization never quite completes...


Version: unspecified
Severity: enhancement

Details

Reference
bz38128

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 12:52 AM
bzimport set Reference to bz38128.
brion created this task.Jul 2 2012, 11:11 PM

Is there a nice way to test for CE functionality automatically, or do we need to create a whitelist-of-doom?

brion added a comment.Jul 2 2012, 11:25 PM

According to http://stackoverflow.com/questions/1882205/how-do-i-detect-support-for-contenteditable-via-javascript you can check for the contentEditable property in the DOM, but this gives false positives on iOS and Android versions where the engine supports it but provides no way to edit things.

So possibly a test combined with a blacklist for Android browser < 3 and iOS < 5?

Now that bug 38857 is fixed, this is easy... Or is it?

This is what I got from $.client.profile() on Jon's phone which runs Android 4.04:

{"name":"safari","layout":"webkit","layoutVersion":534,"platform":"linux","version":"4.0","versionBase":"4","versionNumber":4}

Maybe we could enhance jQuery.client so it can cope with Android user agent strings.

Timo, can you work on getting Android support into jQuery.client so we can close this out?

I'll see if its possible to extend jQuery.client. Otherwise we may want re-consider replacing this module with something more solid.

There's two major parsers I know work very well and have been proven to work by "the mass":

  • browscap [1] (supported by PHP's get_browser functionality)
    • Currently PHP only, and the source is a hugely inefficient .ini file that isn't very future proof. Not the kind of thing we'd want to port to javascript, not to mention the huge manifest.
  • ua-parser [2] (forked from
    • Currently in many languages, including javascript. Though nodejs, we'll have to export the yaml file to JSON and figure out if there is any nodejs modules we need to substitute.

[1] http://browscap.org | https://browsers.garykeith.com/ | https://github.com/GaretJax/phpbrowscap
[2] https://github.com/tobie/ua-parser | https://github.com/tobie/ua-parser/tree/master/js

I submitted a pull request to ua-parser just now.

Once they merge it I'll add that as a new module and let VisualEditor use that instead. I should probably keep jquery.client in core so that extensions using that can continue to do so (maybe as a wrapper though, to avoid repetition).

Having discussed this with Timo just now, ua-parser has now moved even further away from being appropriate for client-side use, so here's the new plan:

  • We will have some basic up-front tests of browser functionality - contentEditable, localStorage, etc.

If the browser is detected to not have this, we will fail with an error message along the lines of "Sorry, this browser does not support some of the core technologies used in VisualEditor and so it will not load. <Oh well>"

  • Then we have two lists - a known-good list (i.e., currently the IE9+/Firefox10+/Chrome16+/Safari5+ list) and a known-bad list (e.g., Android<=2, Opera) If the browser is in the former, VE loads; if in the latter we will fail with an error message along the lines of "Sorry, this browser is known to have serious issues with VisualEditor and so it will not load. <Oh well>"

If we reach this point (seems to have the technologies, not a known-bad or a known-good), we give the user a choice about whether to proceed - "We do not know if this browser works perfectly with VisualEditor; do you wish to proceed? <OK> <Cancel>" - and set the answer in localStorage.

  • Bug 41149 has been marked as a duplicate of this bug. ***

Setting milestone per James.

@Ed Feature tests:

  • Engine is ECMAScript5 compliant.

    Avoid testing things like Object.create or Function#bind which can easily be polyfills (and asserting the difference between native and polifill can be tricky). Assert that strict mode works.

    supportsStrictMode = ( function () { "use strict"; function x() { return this; } return x() === undefined; }() );

    Perhaps you could even take out x() and just return this === undefined. Though that appears to work and seems more straight forward, I see all patterns out there use a function inside. Let's figure out if its needed and only do it if needed.
  • Browser supports ContentEditable.

    DOMElements have all supported properties defined in their default state. e.g. all input fields have property 'placeholder' (empty string as value by default). supportsPlaceholder = 'placeholder' in document.createElement( 'input' );

    For content editable we don't need to create an element, we can just use document.body as test target since it works body as well.

    supportsContentEditable = 'contentEditable' in document.body;

Related URL: https://gerrit.wikimedia.org/r/66993 (Gerrit Change Idc5f4a23a2709264d869a91d00873c4e187bc470)

Related URL: https://gerrit.wikimedia.org/r/67082 (Gerrit Change Ic38992e04b5f3932cf18f2dfc217cd733196efb8)