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.

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

Custom WebVR Polyfill: Error Loading DPDB because of xhr request to already required JSON #2385

Closed
Stefie opened this issue Feb 14, 2017 · 15 comments

Comments

@Stefie
Copy link
Contributor

Stefie commented Feb 14, 2017

Hi,
I'm getting this error-message when I'm opening any Scene that uses A-Frame 0.5.0 on my iPhone 6s.

screen shot 2017-02-14 at 11 55 43

It's a bug in the WebVR Polyfill, but I'm reporting it here because it looks like this fork of the Polyfill is only used for A-Frame:
https://github.com/dmarcos/webvr-polyfill#a02a808 (probably related to #2008 and #2352 )

In @dmarcos version the path to the dpdb.json is relative, not absolute like in the master of the WebVR Polyfill.

The JSON is getting required by var ONLINE_DPDB_URL = require('./dpdb.json'); so ONLINE_DPDB_URL already is the JSON object, not a path to a JSON file like in the polyfill master.

The Bug is that the url in the xhr-request xhr.open('GET', ONLINE_DPDB_URL, true); is the required JSON object - not a URL - which is the cause for the 404 error. There's no need for the xhr-request because the JSON object is already stored in ONLINE_DPDB_URL.

The Bug is in https://github.com/dmarcos/webvr-polyfill/blob/master/src/dpdb/dpdb.js#L22 line 22-61

@Stefie
Copy link
Contributor Author

Stefie commented Feb 14, 2017

Also the commit referenced here "webvr-polyfill": "dmarcos/webvr-polyfill#a02a8089b" in the package.json of a-frame doesn't exist. It should be #a02a808 instead of #a02a8089b.
https://github.com/aframevr/aframe/blob/master/package.json#L43

@dmarcos
Copy link
Member

dmarcos commented Feb 18, 2017

@Stefie The commit referred by the package.json indeed exists dmarcos/webvr-polyfill@a02a8089b which is the same as dmarcos/webvr-polyfill@a02a8089 In git you can refer a commit by just a unique prefix
I can't reproduce the issue on my desktop machine. Can you somehow share a link where the error manifests?

@Stefie
Copy link
Contributor Author

Stefie commented Feb 19, 2017

Hi @dmarcos ,
the bug probably occurs on all mobile devices. I only tested it with my old iPhone 5 and my 6s Plus, but since it fails to load the online DPDB (which should happen on both Android and iOS) you should be able to reproduce it on an Android phone...

On desktop i can reproduce the Bug when I use the Custom User Agent "iPhone" in the 'Responsive Design Mode' in the Firefox Dev Tools. This makes the polyfill return true when running the isMobile and isIOS checks.

  1. Open https://aframe.io/examples/showcase/helloworld/ in Firefox
  2. Open DevTools and enter 'Responsive Design Mode'
  3. Write "iPhone" in the 'Custom User Agent' field
  4. Reload the page

When I do this, I'm getting the same error-messages that I get on my iPhone:

  • Error loading online DPDB
  • And the 404

screen shot 2017-02-19 at 11 44 17

screen shot 2017-02-19 at 11 48 04

@Stefie
Copy link
Contributor Author

Stefie commented Feb 19, 2017

A possible Bugfix when requiring the online DPDB with a relative path:
https://github.com/dmarcos/webvr-polyfill/blob/master/src/dpdb/dpdb.js#L40

  // XHR to fetch online DPDB file, if requested.
  if (fetchOnline) {
    // Set the callback.
    this.onDeviceParamsUpdated = onDeviceParamsUpdated;

    // BUGFIX: Use the JSON object already stored in ONLINE_DPDB_URL 
    // after requiring './dpdb.json' instead of making a XMLHttpRequest with the Object as URL
    this.dpdb = ONLINE_DPDB_URL;
    if (this.dpdb) {
      this.recalculateDeviceParams_();
    } else {
      // Error loading the DPDB.
      console.error('Error loading online DPDB!');
    }
    /**
    var xhr = new XMLHttpRequest();
    var obj = this;

    ////////////////////
    // BUG: This is where the request to [object Object] happens and the cause for the 404
    ///////////////////

    xhr.open('GET', ONLINE_DPDB_URL, true);
    xhr.addEventListener('load', function() {
      obj.loading = false;
      if (xhr.status >= 200 && xhr.status <= 299) {
        // Success.
        obj.dpdb = JSON.parse(xhr.response);
        obj.recalculateDeviceParams_();
      } else {
        // Error loading the DPDB.
        console.error('Error loading online DPDB!');
      }
    });
    xhr.send();
    **/
  }

@dkhan11
Copy link

dkhan11 commented Feb 23, 2017

Got the same error on Chrome running on Android...
Any solution?
Thanks,
DD

@patcat
Copy link
Contributor

patcat commented Feb 26, 2017

I've had the same error on Chrome on Android too, didn't have a chance to find a solution... switched back to A-Frame 0.4.0 instead for the time being, not sure if the error actually breaks anything but was having a different plugin issue which also was fixed by dropping to 0.4.0.

Whatever the issue is, wasn't there in 0.4.0. It doesn't come up on desktop for me. But comes up on Chrome for Android and I think I also saw it when testing on iOS safari.

@machenmusik
Copy link
Contributor

it seems to me that the correct solution is:
(1) the DPDB JSON file that is to be required should be for DPDB_CACHE, not for ONLINE_DPDB_URL.
(2) if the intent is to avoid making an external call and to always use the DPDB cache, the simplest way is to act as if (or actually make sure) fetch_online is set to false; then the XHR will never happen.
(3) if the intent is to still make an external call, then ONLINE_DPDB_URL should be set to the appropriate URL string value.

@maxweber
Copy link

maxweber commented Mar 27, 2017

I have the same issue on Chrome 56.0.2924.87 on Android 6.0.1 (Samsung S5; tested with aframe master and 0.5.0). Also the examples on aframe.io does not work with Chrome 56 only with Chrome Dev, I guess the latter supports the WebVR API and don't have to use the polyfill.

@maxweber
Copy link

maxweber commented Mar 27, 2017

I tried the bugfix suggested by @Stefie and it resolves the issue. But another error occured afterwards:

Uncaught (in promise) DOMException: Failed to load because no supported source was found.

also this call:

https://github.com/aframevr/aframe/blob/v0.5.0/src/core/scene/a-scene.js#L190

failed on my Samsung S5 with Chrome 56. Regrettably, I don't have the exact error message anymore.

I resolved the issue by replacing "webvr-polyfill": "dmarcos/webvr-polyfill#a02a8089b" with "webvr-polyfill": "^0.9.26" in the package.json of the aframe project (master). I used npm link to use my adapted aframe version in my other project.

I don't know the root cause of the problem. I also compared the entry for my device in the https://github.com/googlevr/webvr-polyfill/blob/master/src/dpdb/dpdb-cache.js with the one in the "dmarcos/webvr-polyfill#a02a8089b" fork, but they are the same.

I also tested the current aframe master version (which cause the issue on my device) on a Google Pixel with the same browser version but Android 7 and it runs without problems on the Pixel.

Update: even the fixed version leads to a Chrome crash on my S5. But on a Pixel phone and a Nexus 5x it works fine.

@jbreckmckye
Copy link

jbreckmckye commented Apr 3, 2017

So, basically -

  • we've forked webvr-polyfill because the online DPDB store is outdated, and we don't know who at Google can update it
  • we want to use our own JSON of device params instead (dpdb.json)
  • we're pulling that into dpdb.js with a require: var ONLINE_DPDB_URL = require('./dpdb.json');
  • we're passing that 'URI' (actually an object) to XHR.open. Naturally, this fails, because it is not a string. We end up fetching a URI that ends with '[Object object]'.

I can think of three fixes, in order of work required:

  1. Stop fetching DPDB over the network and just use our up-to-date JSON directly. For instance, we could merge our device data into dpdb-cache.js and instantiate DPDB with its fetchOnline set to false;
  2. Poke / help / bribe / beg someone at Google to make them update their DPDB data, to support more devices, and then get rid of our own device definitions;
  3. Create an open DPDB repository that anyone can submit changes to, serve it somewhere and make a PR so webvr-polyfill uses it.

@jbreckmckye
Copy link

I've created a PR to fix this issue over in the webvr-polyfill fork. If anyone could take the time to look at it, that'd be really helpful: aframevr/webvr-polyfill#3.

@dmarcos
Copy link
Member

dmarcos commented Apr 3, 2017

Cool, thanks! Anyone knows the procedure to update the polyfill hosted on https://storage.googleapis.com/cardboard-dpdb/dpdb.json ? Does any one know who's the point of contact? I don't mind hosting a github.com/aframevr/dpdb.json repo that people can submit PRs against. I want to make sure it's not going to be confusing

@jbreckmckye
Copy link

@dmarcos I believe it's @borismus' team. However -

unfortunately given my team's focus on Daydream, just haven't had time to split things up or add to dpdb.

immersive-web/webvr-polyfill#147 (comment)

@jsantell
Copy link
Contributor

@cvan and I have been working on syncing up @thoragio's initiative at webvrrocks/webvr-polyfill-dpdb which publishes the DPDB at https://dpdb.webvr.rocks/dpdb.json instead of the no-longer-updated https://storage.googleapis.com/cardboard-dpdb/dpdb.json. The latest version of webvr-polyfill is in sync with this, as well as has the correct online URL, so #2700 will fix this issue

@ngokevin
Copy link
Member

#2700 fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants