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

x/playground: reopening closed playground loses content #49895

Closed
rogpeppe opened this issue Dec 1, 2021 · 10 comments
Closed

x/playground: reopening closed playground loses content #49895

rogpeppe opened this issue Dec 1, 2021 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@rogpeppe
Copy link
Contributor

rogpeppe commented Dec 1, 2021

With the old playground, if you close the tab (I do this quite often accidentally when I type ctrl-w trying to delete the last word), reopening it would bring it back without losing any content. You can easily verify this by trying the old playground at go.

In the new playground, the reopened tab reverts the content to its original state, which can be very frustrating if one has spent a while editing there.
@gopherbot gopherbot added this to the Unreleased milestone Dec 1, 2021
@mknyszek
Copy link
Contributor

mknyszek commented Dec 2, 2021

CC @rsc ? Should I be directing this to someone else, or to a group?

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 2, 2021
@rogpeppe
Copy link
Contributor Author

Just bitten by this badly for the nth time after spending 15 minutes editing some code only to lose it. Any chance of a fix, please?

@seankhliao
Copy link
Member

I think it's a race between https://go.googlesource.com/website/+/refs/heads/master/_content/js/playground.js#553 and whatever does the restoring (browser?)

@rogpeppe
Copy link
Contributor Author

Yes, a race sounds about right - I thought I saw my content appear for a moment before before deleted.

@bcmills
Copy link
Contributor

bcmills commented May 18, 2022

(CC @jamalc as someone on the Go team who might know things about browsers)

@ZekeLu ZekeLu mentioned this issue May 19, 2022
Closed
@ZekeLu
Copy link
Contributor

ZekeLu commented May 19, 2022 •
edited
Loading

I have tested this issue with a local modified js file, and it turns out that the culprit is https://go.googlesource.com/website/+/refs/heads/master/_content/js/playsite.js#26. I have sent out a CL to remove this line, hopefully that it will fix this issue.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/407055 mentions this issue: _content: fix go.dev/play not to select example on load

@ZekeLu
Copy link
Contributor

ZekeLu commented May 25, 2022

Hi @jamalc, I found that the CL has been deployed and just had a test. Unfortunately, it doesn't fix this issue reliably. The flow is like this:

  • The "hello world" example is loaded
  • => maybe: the click event of the run button is triggered (case 1)
  • => The user content is restored
  • => maybe: the click event of the run button is triggered (case 2)
  • => The server returns the fmt result
  • => The textarea is updated with the fmt result from the server.

Either case 1 or case 2 will happen, but not both.
In case 1, the source code of the "hello world" example is provided to the server and it will be the final text in the textarea.
In case 2, the user content is provided to the server, and it will be the final text in the textarea.

I think we have to remove $('.js-playgroundRunEl').click(); or delay its execution to avoid the race.

@ZekeLu ZekeLu mentioned this issue May 25, 2022
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/408414 mentions this issue: _content: do not run playground code on page load

@ZekeLu
Copy link
Contributor

ZekeLu commented May 25, 2022 •
edited
Loading

I found another issue caused by https://go.dev/cl/407055. When we open a shared code (for example, https://go.dev/play/p/cnjP1PbosLa). The "Run" action could send Loading share... to https://go.dev/_/fmt, which results in:

prog.go:1:1: expected 'package', found Loading

Go build failed.

I'm very sorry for the error. https://go.dev/cl/408414 will fix this issue too.

@ZekeLu ZekeLu mentioned this issue May 25, 2022
Closed
gopherbot pushed a commit to golang/website that referenced this issue May 25, 2022
@ZekeLu @jamalc
_content: do not run playground code on page load
554288c 
The purpose is to avoid overwriting user content restored by opening
recently closed tab from the history.

It turns out that https://golang.org/cl/407055 does not address the
issue reliably. If the run action happens before the user content is
restored, the source code of the "hello world" example will be sent
to the server for formatting. Most of the time (if not always), the
format result will return after the user content is restored. In this
case, the format result will overwrite the restored user content.

We can delay the run action until the user content is restored. But it's
subtle to decide how long to delay. On the other hand, I think it does
not make sense to run the "hello world" example on load, so I prefer
to address the issue by removing the run action on page load.

Fixes golang/go#49895
Fixes golang/go#53065

Change-Id: Ia044103e79a15d0b351e4257af3aa74558c302d9
GitHub-Last-Rev: 40170ad
GitHub-Pull-Request: #162
Reviewed-on: https://go-review.googlesource.com/c/website/+/408414
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
@golang golang locked and limited conversation to collaborators May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
6 participants
@rogpeppe @ZekeLu @mknyszek @bcmills @gopherbot @seankhliao