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

fix: correctly render existing bonds in Safari #584

Merged
merged 1 commit into from
Feb 1, 2023

Conversation

theborakompanioni
Copy link
Collaborator

@theborakompanioni theborakompanioni commented Jan 3, 2023

It seems Safari is not able to construct Date objects from strings formatted as yyyy-MM-dd 00:00:00 (see stackoverflow#4310953).
Before this commit, as soon as a user on Safari created a fidelity bond, an error was thrown and the page became Earn page did not render.
After this commit, the fidelity bond is shown correctly and the Earn page renders successfully.

This is an untested change, as every attempt to run Safari failed on my workstation. It be great if someone can verify that it works as expected. However, the root problem should be addressed, as all date objects are instantiated with a timestamp (type number) which every browser supports.

@theborakompanioni theborakompanioni added the bug Something isn't working label Jan 3, 2023
@theborakompanioni theborakompanioni self-assigned this Jan 3, 2023
@theborakompanioni theborakompanioni changed the title fix: correctly render exsting bonds in Safari fix: correctly render existing bonds in Safari Jan 3, 2023
@theborakompanioni theborakompanioni marked this pull request as ready for review January 3, 2023 17:44
@dergigi
Copy link
Contributor

dergigi commented Jan 23, 2023

I was able to create a FB without issues on current master commit 9d1575f running Safari Version 16.2 (18614.3.7.1.5).

Thus running the changeset of this PR has no effect for me, it seems?


Edit: It always works and looks like this:

Screenshot 2023-01-23 at 18 47 02

@theborakompanioni
Copy link
Collaborator Author

theborakompanioni commented Jan 24, 2023

I was able to create a FB without issues on current master commit 9d1575f running Safari Version 16.2 (18614.3.7.1.5).

Just fyi, this has been reported by two distinct users on Safari. After creating a FB, Jam "crashed" (showing the error page). Switching to a different browser has been a successful workaround. Unfortunately, I do not know which version of Safari they used.

Additionally, it is still safer to use the unix timestamp appended to the path property of the UTXO object, than to pass the raw locktime property to the Date constructor. e.g.

{
  [...]
  "path": "m/84'/1'/0'/2/40:1682899200",
  "locktime": "2023-05-01 00:00:00",
}

That's why it might be good to be merged.
What do you think?

Copy link
Contributor

@dergigi dergigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, tACK ✅

(even though I wasn't able to reproduce the bug, as mentioned above; but it doesn't break anything, so it should be good to go)

@dergigi dergigi merged commit 9ebe168 into master Feb 1, 2023
@dergigi dergigi deleted the fix/safari-date-parsing branch February 1, 2023 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants