New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve custom-protocol-handler.js #992

Merged
merged 1 commit into from Mar 7, 2014

Conversation

Projects
None yet
2 participants
@patrickkettner
Member

patrickkettner commented Jul 15, 2013

As documented in the Undetectables
navigator.registerProtocolHandler was stubbed in webkit for a while, and doesn't actually do anything there.
This test forces a failure and ensures it's the right type of failure.

Android 2.* showed this behavior previously, and this test fixes that false positive.

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Mar 1, 2014

aye yo @stucox

catch (e) {
return e instanceof TypeError;
}

This comment has been minimized.

@stucox

stucox Mar 4, 2014

Member

Fancy sticking a return false; here, just in case some browser for some reason doesn’t throw an error on L32?

This comment has been minimized.

@patrickkettner

patrickkettner Mar 4, 2014

Member

pah. morning brain. thanks so much stu

On Tue, Mar 4, 2014 at 9:43 AM, Stu Cox notifications@github.com wrote:

In feature-detects/custom-protocol-handler.js:

*/
define(['Modernizr'], function( Modernizr ) {

  • Modernizr.addTest('customprotocolhandler', !!navigator.registerProtocolHandler);
  • Modernizr.addTest('customprotocolhandler', function() {
  • // early bailout where it doesn't exist at all
  • if ( !navigator.registerProtocolHandler ) return false;
  • // registerProtocolHandler was stubbed in webkit for a while, and didn't
  • // actually do anything. We intentionally set it improperly to test for
  • // the proper sort of failure
  • try {
  •  navigator.registerProtocolHandler('thisShouldFail');
    
  • }
  • catch (e) {
  •  return e instanceof TypeError;
    
  • }

Fancy sticking a return false; here, just in case some browser for some
reason doesn't throw an error on L32?

Reply to this email directly or view it on GitHubhttps://github.com//pull/992/files#r10265252
.

patrick

improve custom-protocol-handler.js
As documented in the Undetectables -
https://github.com/Modernizr/Modernizr/wiki/Undetectables -
navigator.registerProtocolHandler was stubbed in webkit for a while, and
doesn't actually do anything there. This test forces a failure and ensures it's
the right type of failure.
@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Mar 6, 2014

updatleydoodled

@stucox

This comment has been minimized.

Member

stucox commented Mar 6, 2014

Good things.

Minor thing, for discussion really – I wonder if we should avoid linebreaks (except where we want paragraph breaks) in our DOC block – seeing that we extract those and push them through a Markdown parser, I think line breaks might be interpreted as new paragraphs. Although I haven’t checked this and probably should have done before saying this.

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Mar 6, 2014

seems like they are

simple fix, will PR soon. this is good to merge?

patrickkettner added a commit that referenced this pull request Mar 7, 2014

@patrickkettner patrickkettner merged commit 30b5d55 into Modernizr:master Mar 7, 2014

1 check passed

default The Travis CI build passed
Details

@patrickkettner patrickkettner deleted the patrickkettner:registerProtocolHandler branch Mar 7, 2014

@stucox

This comment has been minimized.

Member

stucox commented Mar 7, 2014

👍

@stucox stucox referenced this pull request May 21, 2014

Closed

v3.0 release notes #805

patrickkettner added a commit to patrickkettner/Modernizr that referenced this pull request Feb 22, 2015

Merge pull request Modernizr#992 from patrickkettner/registerProtocol…
…Handler

improve custom-protocol-handler.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment