Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Doesn't work on Windows properly #14

Open
dahei opened this issue Jun 9, 2016 · 3 comments
Open

Doesn't work on Windows properly #14

dahei opened this issue Jun 9, 2016 · 3 comments

Comments

@dahei
Copy link

dahei commented Jun 9, 2016

Problem:

Using this plugin under Windows, the linting fails because PhantomJS just returns

<html><head></head><body></body></html>

As it seems, PhantomJS doesn't handle full Windows paths like C:\foo\bar\index.html which gets returned from the file.path

Possible Solution:

I could make it work by hacking https://github.com/yargalot/gulp-accessibility/blob/master/index.js#L33

if (file.isBuffer()) {
      return accessSniff
        .default(fileUrl(file.path), gulpOptions) // using file-url to convert Windows path to classic file URI
        .then(function(response) {
          file.contents = new Buffer(JSON.stringify(response));

          return callback(null, file);
        })
        .catch(function(error) {
          var error = new Error(error);

          return callback(error, file);
        });
    }

You could use https://github.com/sindresorhus/file-url

The file path gets converted to something like

file:///C:/foo/bar/index.html

which is fine for PhantomJS.

I haven't tested this conversion on OS X/Linux, but it shouldn't do no harm.

Maybe there a other solutions, it was the easiest way to fix it for me.
@dahei dahei changed the title Doesn't work on Window properly Doesn't work on Windows properly Jun 9, 2016
@Erbenos
Copy link

Erbenos commented Aug 22, 2016

Noticed this too. Exactly same issue.

@jamiepreston
Copy link

I was experiencing the same issue, thanks for the fix! A pull request for this addition would probably save some headaches - how about it?

@yargalot
Copy link
Owner

yargalot commented Sep 2, 2016

Hey guys, sorry about the delay here, im actually travelling atm so wont be able to get this out till around the 28th. Will release it asap and put some tests around it when i get back.

@fridagoransson fridagoransson mentioned this issue Jun 9, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@yargalot @dahei @Erbenos @jamiepreston