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

Unable to Create New Page In Mobile Browser #93852

Closed
cat-og opened this issue Aug 23, 2024 · 16 comments · Fixed by Automattic/jetpack#39768
Closed

Unable to Create New Page In Mobile Browser #93852

cat-og opened this issue Aug 23, 2024 · 16 comments · Fixed by Automattic/jetpack#39768
Assignees
Labels
[Feature Group] Editor Experience Features related to Gutenberg integration on WordPress.com. [Feature] Post/Page Editor The editor for editing posts and pages. [Platform] Atomic [Platform] Simple [Pri] High Address as soon as possible after BLOCKER issues [Product] WordPress.com All features accessible on and related to WordPress.com. [Status] Priority Review Triggered Quality squad has been notified of this issue in #dotcom-triage-alerts [Type] Bug When a feature is broken and / or not performing as intended

Comments

@cat-og
Copy link

cat-og commented Aug 23, 2024

Quick summary

I am unable to create a new page in my mobile browser. Tested in both Safari and Chrome; when I create a new page from the Dashboard, it takes a moment to load. The patterns appear briefly, and then the page refreshes itself. The patterns appear again for a short time, and then the entire page closes and I see a browser error.

iPhone 15
iOS 17.5.1
Tested in Chrome and Safari

Steps to reproduce

  1. Start on mobile Dashboard in a browser, not the Jetpack app
  2. Use mobile Dashboard navigation to Create a New Page
  3. The new page will not load, and the browser will eventually return an error message.

What you expected to happen

A new post will be created which I can then edit and publish.

What actually happened

The browser returns an error message. If I hit back from there, it brings me to the last post I was working on, not back to the Dashboard.

RPReplay_Final1724395967.MP4

Impact

Most (> 50%)

Available workarounds?

No but the platform is still usable

If the above answer is "Yes...", outline the workaround.

No response

Platform (Simple and/or Atomic)

Simple, Atomic

Logs or notes

No response

@cat-og cat-og added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Post/Page Editor The editor for editing posts and pages. Needs triage Ticket needs to be triaged [Product] WordPress.com All features accessible on and related to WordPress.com. [Feature Group] Editor Experience Features related to Gutenberg integration on WordPress.com. labels Aug 23, 2024
@github-actions github-actions bot added [Status] Priority Review Triggered Quality squad has been notified of this issue in #dotcom-triage-alerts [Platform] Atomic [Platform] Simple [Pri] High Address as soon as possible after BLOCKER issues labels Aug 23, 2024
@xavier-lc xavier-lc self-assigned this Aug 23, 2024
@TimBroddin TimBroddin moved this from Needs Triage to In Triage in Automattic Prioritization: The One Board ™ Aug 23, 2024
@TimBroddin
Copy link
Contributor

📌 REPRODUCTION RESULTS

I can reproduce.

📌 FINDINGS/SCREENSHOTS/VIDEO
Same results as above.

@TimBroddin TimBroddin added the Groundskeeping Issues handled through Dotcom Groundskeeping rotations label Aug 23, 2024
@xavier-lc
Copy link
Contributor

Unassigning myself, I only own Android devices and the flow works fine on them.

@xavier-lc xavier-lc removed their assignment Aug 23, 2024
@TimBroddin TimBroddin self-assigned this Aug 23, 2024
@TimBroddin
Copy link
Contributor

I'm having a hard time remote debugging this on my iPhone, but my hunch is that this is an OOM error, caused by rendering the block previews.

The heap sizes of Calypso + editor iframe + block previews probably overflows the max memory allowed (which depending on source is somewhere between 384 and 500 MB).

The <BlockPreview /> component lives in Gutenberg: https://github.com/WordPress/gutenberg/tree/trunk/packages/block-editor/src/components/block-preview.

The modal lives here: https://github.com/Automattic/wp-calypso/tree/trunk/packages/page-pattern-modal

@p-jackson (don't bother reading this until you get back from leave): as the author of the pattern modal, do you have any idea on how to best resolve this?

@TimBroddin TimBroddin moved this from In Triage to Triaged in Automattic Prioritization: The One Board ™ Aug 23, 2024
@Imran92
Copy link
Contributor

Imran92 commented Aug 26, 2024

I'm having a hard time remote debugging this on my iPhone, but my hunch is that this is an OOM error, caused by rendering the block previews.

The heap sizes of Calypso + editor iframe + block previews probably overflows the max memory allowed (which depending on source is somewhere between 384 and 500 MB).

The <BlockPreview /> component lives in Gutenberg: https://github.com/WordPress/gutenberg/tree/trunk/packages/block-editor/src/components/block-preview.

The modal lives here: https://github.com/Automattic/wp-calypso/tree/trunk/packages/page-pattern-modal

@p-jackson (don't bother reading this until you get back from leave): as the author of the pattern modal, do you have any idea on how to best resolve this?

Wondering if #83472 has fixed this issue as well

@Imran92
Copy link
Contributor

Imran92 commented Aug 26, 2024

Removing the "Needs Triage" label as I was able to reproduce it in my phone as well following the instructions

WhatsApp.Video.2024-08-26.at.15.21.12.mp4

@Imran92 Imran92 removed the Needs triage Ticket needs to be triaged label Aug 26, 2024
@miksansegundo
Copy link
Contributor

miksansegundo commented Sep 6, 2024

I can also reproduce it on a real iOS device. It looks like a memory issue. It doesn't happen all the times and seems to be triggered by the scroll.

Note this modal has been migrated from the ETK plugin to the Jetpack-mu-wpcom plugin and uses the npm package page-pattern-modal. See /features/starter-page-templates/

I haven't found any clue on bottlenecks or memory leaks when doing a performance and a memory analysis in Chrome Dev tools.
I'm going to use my sandbox to test some changes to try to reduce the scope of the code causing this bug.

@miksansegundo
Copy link
Contributor

This looks like a rendering issue with a memory leak. I haven't reproduced the page crash when the page loads but I can reproduce it by just doing clicks on the screen, no need to scroll.

Screen.Recording.2567-09-06.at.18.14.11.mov

I'll continue debugging the rendering in Chrome Dev Tools next week. I expect to find a React memo that is not working as expected or it's missing to prevent unnecessary renders.

@miksansegundo miksansegundo self-assigned this Sep 6, 2024
@miksansegundo miksansegundo moved this from Triaged to In Progress in Automattic Prioritization: The One Board ™ Sep 6, 2024
@miksansegundo
Copy link
Contributor

miksansegundo commented Sep 10, 2024

This issue is affecting the pattern rendering in all the editors. Safari mobile crashes when exploring patterns in the page, post and site editors.

I have created an issue in Gutenberg to address the pattern rendering performance issue there.

Edit: The issue could be in some plugin in the Dotcom side because I cannot reproduce it on a core site. Maybe it's because of the number of patterns because a Dotcom site has many more patterns from our library.

@miksansegundo miksansegundo moved this from In Progress to Needs Core/3rd Party Fix in Automattic Prioritization: The One Board ™ Sep 10, 2024
@miksansegundo miksansegundo added the [Status] Core Fix Needed A fix within the Core WordPress or Gutenberg project is required to resolve this issue. label Sep 10, 2024
@miksansegundo miksansegundo removed the [Status] Core Fix Needed A fix within the Core WordPress or Gutenberg project is required to resolve this issue. label Sep 11, 2024
@miksansegundo
Copy link
Contributor

@TimBroddin I have found that reverting the diff D109323-code to use the map provider mapbox rather than mapkit (which uses Apple maps in iOS) improves the pattern rendering performance a lot and helps prevent the browser crash. Do you know the context of using mapkit?

Using Mapbox won't fix the issue totally but it helps a lot. Otherwise, Safari crashes every time a pattern with an Apple map is rendered. Sometimes on the first load, others on the second load but it always crashes.

I believe the root cause is multiple issues because of the features Dotcom adds to the editor. I'm still debugging the features in the plugin jetpack-mu-wpcom one by one to find more issues.

The modal itself can also use more memoizations. I'll follow up with more later with a PR.

This GB performance issue can affect here too.

@TimBroddin
Copy link
Contributor

@miksansegundo Thanks for digging into this.

The main reason for switching the map block to Mapkit was savings. We spent a ton on Mapbox, and Mapkit is free.

We could use Mapbox or a static image when rendering the pattern selector if we can somehow determine that we're being rendered there. The downside would be that the map would look different on insertion. I can look into that if you want.

@p-jackson
Copy link
Member

Would a quick fix simply be to bypass the modal on mobile devices and just start with a blank page?
It feels warranted given this currently blocks the "create new pager" flow on mobile.

@p-jackson (don't bother reading this until you get back from leave): as the author of the pattern modal, do you have any idea on how to best resolve this?

Things have definitely changed in this modal since it was first created. A lot about patterns has changed. It originally showed jpeg screenshots of the patterns which is much more memory friendly. Check out this page 359fb-pb/. But this was all before patterns were in the core editor.

Now the block inserter menu shows live previews of patterns, and using mShots isn't an approach that would work for Core. By using dynamic previews it means the previews are more easily translatable and can use the correct styling based on theme.json. Using an mShot approach would mean taking jpeg screenshots of many many permutations of each pattern.

But yeah, all those iframes are pretty heavy 😬

I was just testing the block inserter menu on my phone, and even some of those pattern previews cause the site to crash for me!

@TimBroddin
Copy link
Contributor

TimBroddin commented Sep 13, 2024

I did some digging and using something like this works:

useEffect(() => {
		if ( document.querySelectorAll('body.modal-open').length > 0 ) {
			setIsInsidePatternList(true);
		}
	}, []);

if ( preview ||  isInsidePatternList ) {
		const mapStyleObject = styles.find( styleObject => styleObject.name === mapStyle );

		content = (
			<div className="wp-block-jetpack-map__map_wrapper">
				<img
					alt={__('Map Preview', 'jetpack')}
					src={mapStyleObject ? mapStyleObject.preview : previewPlaceholder}
					style={{ width: '100%', height: 'auto' }}
				/>
			</div>
		);
...
}

(Note that there's already a preview attribute that triggers the display of a jpg instead of a full map here).

However, it would be great if there was a hook useIsInsideBlockPreview that would get this value from a provider higher up. Checking the existence of a class name feels hacky.

@miksansegundo miksansegundo removed their assignment Sep 17, 2024
@obenland
Copy link
Member

@TimBroddin Any update on this?

@annezazu
Copy link

annezazu commented Oct 7, 2024

Noting that in the future, Core is headed towards removing the modal and instead rely on adding patterns directly from an Inserter: WordPress/gutenberg#63865 In the meantime though, this needs a fix.

@TimBroddin
Copy link
Contributor

@TimBroddin Any update on this?

If this pattern picker is going to go away in the future, the hacky way I suggested above might be a viable option 👍

@zaguiini zaguiini self-assigned this Oct 14, 2024
@davemart-in
Copy link
Contributor

Removing this from The One Board and removing the Groundskeeper label since Quake team took ownership.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature Group] Editor Experience Features related to Gutenberg integration on WordPress.com. [Feature] Post/Page Editor The editor for editing posts and pages. [Platform] Atomic [Platform] Simple [Pri] High Address as soon as possible after BLOCKER issues [Product] WordPress.com All features accessible on and related to WordPress.com. [Status] Priority Review Triggered Quality squad has been notified of this issue in #dotcom-triage-alerts [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants