phetsims / natural-selection

"Natural Selection" is an educational simulation in HTML5, by PhET Interactive Simulations
GNU General Public License v3.0
3 stars 7 forks source link

Example HTML on Safari is all on one line #288

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

Related to https://github.com/phetsims/QA/issues/662.

https://github.com/phetsims/gravity-and-orbits/issues/398 is also a problem in the Natural Selection 1.4, and will need to be patched there. Natural Selection will also require another RC test after https://github.com/phetsims/QA/issues/662.

@samreid @KatieWoe FYI.

pixelzoom commented 3 years ago

Fixing this also involves https://github.com/phetsims/studio/issues/234 (Migrate from highlight.js to prism.js ).

samreid commented 3 years ago

https://github.com/phetsims/studio/issues/234 is fixed and ready for cherry picking. I'll work out the necessary SHAs in https://github.com/phetsims/gravity-and-orbits/issues/398 and report them here once ready.

pixelzoom commented 3 years ago

@samreid is going to handle patching this in natural-selection, since the patch should be the same as GAO.

samreid commented 3 years ago

I followed this procedure to apply the cherry-pick commits for https://github.com/phetsims/studio/issues/234

cd ../perennial
grunt checkout-target --repo=natural-selection --target=1.4

# in safari, test the index wrapper to confirm the problem

cd ../sherpa
git checkout -b natural-selection-1.4
git cherry-pick b74635e2351551e81b5035013e1eb48fda493ae2
git push --set-upstream origin natural-selection-1.4

cd ../studio
git checkout -b natural-selection-1.4
git cherry-pick eaf319c4d9bb4be6b341eda24026c45eac3131c6
git cherry-pick 218d2547cbf3e0a1e964586a1b66058fd118c9e1
git push --set-upstream origin natural-selection-1.4

cd ../phet-io-wrappers
git checkout -b natural-selection-1.4
git cherry-pick 634ffeadb16b64930db510482c9e9a9640ed316b
git push --set-upstream origin natural-selection-1.4

cd ../chipper
git checkout -b natural-selection-1.4
git cherry-pick 99e1abb571dc62d37aa07449c64d93dd3fb51f49
git cherry-pick b8e0a0d630db8e9c42e96685d79e4c330ed2d314
git push --set-upstream origin natural-selection-1.4

# Note this next one is a workaround for the problems experienced in https://github.com/phetsims/chipper/issues/1056
# I'm not planning to push this commit.
git cherry-pick f0a241d3341b16e5ad3a23b0fd311fd5d25044f5

cd ../natural-selection
grunt --brands=phet,phet-io
cp build/phet/dependencies.json .
git add dependencies.json
# Manually update chipper SHA with e0585a3762e3e6aa929eb4999d6aeb0911b3a468 which is one SHA before the workaround above.
git commit -m "Update dependencies.json with SHAs for cherry picks for syntax highlighter https://github.com/phetsims/natural-selection/issues/288"
git push origin 1.4

# Test index wrapper in Safari to make sure it works as expected.
# Test studio => preview HTML in Safari to make sure it works as expected.

Cherry-picks complete and this should be ready for testing in the next RC. Back to @pixelzoom for review or to label/plan accordingly.

samreid commented 3 years ago

I'm having some problems here:

  1. Somehow the later chipper sha got into dependencies.json (from the workaround above). I manually corrected it and committed and pushed. I have an uneasy feeling about this since I don't know how the wrong SHA ended up in dependencies.json.
  2. On correcting that, I accidentally put the wrong commit message, so it didn't show up in this issue thread. The commit was https://github.com/phetsims/natural-selection/commit/2b9411eeeb3069db6612939d77d5e2916d6b8c81

My apologies for the problems. I tested that grunt checkout-target --repo=natural-selection --target=1.4 is working OK, at least on my copy. Can you please test and make sure everything is working OK on your side?

pixelzoom commented 3 years ago

@samreid:

Somehow the later chipper sha got into dependencies.json ...

I don't know how I can confirm that the chipper sha is now correct, so I'll take your word for it.

On correcting that, I accidentally put the wrong commit message, so it didn't show up in this issue thread. ...

You can always go to the commit in GitHub, and add a comment there with the correct issue number.

I tested that grunt checkout-target --repo=natural-selection --target=1.4 is working OK, at least on my copy. Can you please test and make sure everything is working OK on your side?

Did you also build and test that the problem is indeed fixed in the branch?

Looking at the procedure in https://github.com/phetsims/natural-selection/issues/288#issuecomment-874349961... Why was --set-upstream needed? That's not part of the maintence protocol described in Manual maintenance patching.

pixelzoom commented 3 years ago

I did:

% cd perennial
% grunt checkout-target --repo=natural-selection --target=1.4
% cd natural-selection
% grunt --brands=phet,phet-io

I navigated to the built wrapper index, and the example HTML (in the "Wrapper Development" section) now appears to be formatted, not all on one line. Screenshot below.

screenshot_1047
samreid commented 3 years ago

I don't know how I can confirm that the chipper sha is now correct, so I'll take your word for it.

I confirmed the chipper branch had the correct commits, then I took the SHA from the latest commit.

You can always go to the commit in GitHub, and add a comment there with the correct issue number.

Good idea, I'll try to remember that next time.

Did you also build and test that the problem is indeed fixed in the branch?

Yes, I tested built and unbuilt in the branch, and the behavior was as desired.

Why was --set-upstream needed? That's not part of the maintence protocol described in [...]

I don't have my console log any more, but I believe what happened is that the manual says to use git push origin chains-1.2, but I probably incorrectly ran git push instead. Then git told me it didn't know which remote to push to, and asked if I wanted to use git push --set-upstream origin natural-selection-1.4, so I went with that. Next time I'll try to use git push origin natural-selection-1.4 as prescribed in the manual. If I understand the documentation, the --set-upstream only affects my local git repositories and will not affect others.

pixelzoom commented 3 years ago

This seems ready for testing in the next 1.4 RC.

To verify, open the Wrapper index. Look at the "Wrapper Development" section. Verify that the example html is not all on one line.

KatieWoe commented 3 years ago

Looks good in RC.2 on both iPad and MacOS 11