GoogleChrome / dialog-polyfill

Polyfill for the HTML dialog element
BSD 3-Clause "New" or "Revised" License
2.45k stars 245 forks source link

Error with PhantomJS 2.1, with gulp and dialog-polyfill version 0.4.8 is included #154

Closed Csini closed 7 years ago

Csini commented 7 years ago
14 08 2017 15:08:16.018:WARN [watcher]: Pattern ".../**/*.specs.js" does not match any file.
14 08 2017 15:08:23.263:INFO [karma]: Karma v0.13.22 server started at http://lo
calhost:9876/
14 08 2017 15:08:23.268:INFO [launcher]: Starting browser PhantomJS
14 08 2017 15:08:24.945:INFO [PhantomJS 2.1.1 (Windows 7 0.0.0)]: Connected on s
ocket 0t4O_0jxljFDF5Q8AAAA with id 1834520
PhantomJS 2.1.1 (Windows 7 0.0.0) ERROR
  TypeError: undefined is not an object (evaluating 'methodDescriptor.get')
  at .../tmp_libs.js:91970
PhantomJS 2.1.1 (Windows 7 0.0.0) ERROR
  TypeError: undefined is not an object (evaluating 'methodDescriptor.get')
  at .../.tmp/test/tmp_libs.js:91970

in that line is:

Line 91969:      var methodDescriptor = Object.getOwnPropertyDescriptor(HTMLFormElement.prototype, 'method');
Line 91970:      var realGet = methodDescriptor.get;
Csini commented 7 years ago

with version 0.4.7 isn't the error comming...

samthor commented 7 years ago

Yup, because in 0.4.8 we try to polyfill HTMLFormElement to support method="dialog". We don't do that in 0.4.7 so that will work for now.

Out of curiosity, why do you include

in PhantomJS? It's weird that standard HTML features aren't available, like HTMLFormElement.

On 14 August 2017 at 06:37, Csini notifications@github.com wrote:

with version 0.4.7 isn't the error comming...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/GoogleChrome/dialog-polyfill/issues/154#issuecomment-322191871, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHRkBNXaTfdgtqbUxW2Q1EW-07PQ9gXks5sYE19gaJpZM4O2WRp .

Csini commented 7 years ago

I just run a "normal" unit-test for angularjs with karma, for example:

describe('component: blaBlaHelper', function() {
    var vm, scope, log;

    beforeEach(module('at.blabla'));

    beforeEach(inject(function ($rootScope, _$componentController_, _$log_) {
        scope = $rootScope.$new();
        vm = _$componentController_('blaBlaHelper', {$scope: scope}, {});
        log = _$log_;
        log.reset();
    }));

    afterEach(function () {

    });

    describe('dummy', function () {
        it('vm.var to be Defined', function () {
            expect(true).toEqual(true);
            console.log('Test dummy() finished');
        });
    });
});

-> all the main files from my bower_components are concatenated and loaded (configured in karma-config) to be availible while the test ist running (dialog-polyfill is one of them) -> if I run the test with Karma with Chrome, there isn't any error -> if I run it with PhantomJS (for example to monitor it better on Jenkins) than comes the error that is described above (methodDescriptor is undefined)

Csini commented 7 years ago

is it a good idea to make such a change with a minor version? it destroyed my build for days...

your library was defined in my bower.json so:

"dependencies": {
    "dialog-polyfill": "0.4.*",
     ...
}
samthor commented 7 years ago

Looks like this is a problem with PhantomJS. I'd guess that it's fixed if you upgrade it to 2.5.

I'll add a workaround anyway, but it's a stopgap.

samthor commented 7 years ago

Please try the version of dialog-polyfill.js now checked in at HEAD and let me know if it fixes the problem.

maggo commented 7 years ago

Same problem under iOS 9: Object.getOwnPropertyDescriptor(HTMLFormElement.prototype, "method") returns undefined and kills the dialog functionality

samthor commented 7 years ago

@maggo can you try the version at HEAD?

maggo commented 7 years ago

@samthor yep error is gone, thanks

samthor commented 7 years ago

I've published 0.4.9 to work around this bug. It's still a bug, as these older browsers won't correctly return dialogMethodForm.method as "dialog".

maggo commented 7 years ago

Ah, classic. Thanks for the quick fix!

Csini commented 7 years ago

Woow you are very quick! Thank you very much!! With Version 0.4.9 works my test again! I will check also if we can update our PhantomJS version.