Safari: RangeError: Maximum call stack size exceeded.

[karlwhite]

When using the overlay-datetime-picker on Safari 11.0.1, the browser throws a RangeError: Maximum call stack size exceeded. error (after freezing the browser for some time).

I tried simpler and simpler implementations, and found that it will even occur by placing <time-input></time-input> on a page.

After a cursory poke around the debugger, it appears that the recursion may be caused by the datetime-mixin _computeValue method, when this.setDate is called, but I don't know this code well enough to determine if that really is the cause.

[karlwhite]

A little more info... it appears that polyfill-picker-pattern->_computeValue and datetime-mixin->setDate are both causing each other to fire during their calls to this.setProperties, hence the recursion.

[fooloomanzoo]

I can not debug on Safari. Does the mistake occure from startup or when you change the input? Since polyfill-picker-pattern is only affecting the confirmed-attributes and the mixin isn't used by <time-input> too, the problem calling the setters might be caused by something different. in datetime-mix->setDate is checked if the date really changed, there could be something wrong for formatting or getting the date.

[fooloomanzoo]

A guess could be that the TimezoneOffset is not picked by safari from your local offset, but I investigate

[karlwhite]

Ah... yes, I think you might be on to something. If I drop a breakpoint into _setDate, then I do see the initial value changing as if each iteration is being changed by the timezone offset.

And it occurs from startup (and without any initial value specified), so I guess when it is initialized with the current datetime the same issue applies.

[fooloomanzoo]

A possible mistake, I figured out, could be, that the max-attribute of the input for day is acting non-inclusive, because of the property-effects of polymer itself, what causes the day-input to be reset to the range-borders by my <number-input>-element and we are at the edge of a month. So I increased that value by one and decreased the min-attribute by one to zero, so that it acts more stable. The TimezoneOffset might not be the cause, because it is according to mdn not settable. But in someway that should be also usable for this element. This effect you are noticing also occures on firefox and chrome. Could you please update the element to the new version, and see if the fix works for you?

[karlwhite]

No dice :( I'm still seeing the same behavior.

If I place a breakpoint inside polyfill-picker-pattern->_computeValue then with each recursion/iteration I see the time value being decremented by 7 hours, which just so happens to be my current timezone offset (i.e. PDT = GMT-7), so I think that brings things back to timezone offset being the cause.

[karlwhite]

Sooo... I have found a workaround (super hacky, but confirms the offset being the issue)...

In datetime-mixin->_setDate method, where the date & time values are extracted, instead of fetching the values from the Date() object, I instead fetch them from the ISO String version of that same object (so it is already converted to UTC). Here is the change I made inside _setDate...

const value = +d,
   date = d.toISOString().split('T')[0],  // this._toDate(d),
   time = d.toISOString().split('T')[1].split('Z')[0],  //this._toTime(d),
   datetime = date + 'T' + time;

This gets things working on Safari (while still working properly on Chrome), so hopefully will point you in the direction of a "proper" fix :)

[karlwhite]

Ah... my mistake, the above does break Chrome (with this, Chrome exhibits the same behavior as Safari did previously), so my even hackier workaround for now is the following:

var value = +d,
   date = this._toDate(d),
   time = this._toTime(d),
   datetime = date + 'T' + time;

if ( navigator.userAgent.indexOf('Safari') != -1 && navigator.userAgent.indexOf('Chrome') == -1 ) {
   date = d.toISOString().split('T')[0];
   time = d.toISOString().split('T')[1].split('Z')[0];
   datetime = date + 'T' + time;
}

[fooloomanzoo]

I added in the current new version to every string-based creation of a DateObject the current timeOffset. Chrome does accept that too, but just not for the <input type="datetime-local"/>-element and so timeOffset is now a new attribute and is not added to datetime-attribute, but it is extra.

      _computeValue(date, time) {
        if (date === undefined && time === undefined) return;
        if (this.timezoneOffset === undefined) {
          this._setTimezoneOffset(this._computeTimezoneOffset((new Date()).getTimezoneOffset()));
        }
        if (typeof date === 'object') {
          // 'date' is a Date Object
          this._setDate(date);
        } else if (typeof date === 'string') {
          if (date.match(/[T]/i)) {
            // 'date' is in datetime format
            this._setDate(new Date(date + this.timezoneOffset));
          } else {
            // 'date' is in date format
            this._setDate(new Date(date + 'T' + (time || this.time || '00:00:00') + this.timezoneOffset));
          }
        } else if (typeof time === 'string') {
          // 'date' is not defined but time
          this._setDate((this.date || new Date(this._toDate(new Date())) + 'T' + time + this.timezoneOffset));
        }
      }

It doesn't seem to intended that you can actually set the timeOffset as a user, because the DateObject does only have a getter, so the local timezone is always bound to the DateObject. And so because I guess it would require to almost write an own library to convert timezones to utc-time or this element would need a complete new logic, it is at first just readOnly and is from an initial test. To actually can use the timeOffset in a more handy way, I added also the attribute confirmed-value, from which it should directly be possible to reconstruct the standard time. But hopefully that does solve this issue with Safari?

[fooloomanzoo]

@karlwhite Could you confirm or deny that the changes work on Safari, please?

[karlwhite]

Hey, yes I just confirmed and it's working across both Chrome and Safari!

[fooloomanzoo]

Great! In further versions I work on making the time zone changeable. Thank you!

[fooloomanzoo]

@karlwhite there are now options to set the time-zone. If you add with-timezone there will be input fields for the timezone. Otherwhise timezone is settable and is automatically the local timezone of the user.