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

node.context.InBackend is set wrong when previewing content #3019

Closed
klfman opened this issue Jul 24, 2020 · 29 comments
Closed

node.context.InBackend is set wrong when previewing content #3019

klfman opened this issue Jul 24, 2020 · 29 comments

Comments

@klfman
Copy link

klfman commented Jul 24, 2020

Description

In the Neos backend I often change the rendering of node types to make it easier for the editor to see what they are editing. When they click the "preview" button in the top right of the Neos backend, they expect to see the page as a visitor of their page would see it.
However, in certain cases the value of node.context.InBackend is true when previewing content. As I use this value in a Neos.Fusion:Case to determine which rendering should be used (the Neos backend- or the frontend-representation) the content looks "broken" (a.k.a like in the Neos backend) when previewing.

I made some tests and found out that this phenomenon appears in cases where the preview url contains the username of the currently logged in user in the backend. Two versions of urls seem to be generated, when editors hit the "preview" button in the top right.

  1. http://localhost:8081/foo/bar.html
  2. http://localhost:8081/foo/bar@user-admin.html

In case 1) everything looks fine and node.context.InBackend is false.
In case 2) node.context.InBackend is unexpectedly true, which breaks my content that relies on that detection mechanism.

A thing that I cannot pinpoint is: When and why is @user-admin (or any other user like admin) added to the URL. It seems to be added only sometimes.

Steps to Reproduce

  • Log into Neos
  • Open the preview
  • If the url does not contain the user-username part, just add it manually
  • node.context.InBackend is true, even if we are looking at a frontend preview

Expected behavior

  • node.context.InBackend should never be true in the frontend preview

Affected Versions

  • Neos 4.3.15 & Flow 5.3.15
  • I have no hint when that behavior was introduced
@klfman
Copy link
Author

klfman commented Jul 24, 2020

If that's hard to fix right now, I would love a discussion about workarounds and if they are valid:

  • Can I prevent Neos from rendering the url with @User-username in the preview?
  • Can I detect frontend and preview modes independently from the values of node.context.InBackend and node.context.InFrontend? Maybe I shouldn't be switching my rendering just based on these values?
  • Would it be okay to put a workaround into the Neos core until this issue is fixed, so node.context.InBackend is false for all frontend-previews of the page, even if we cannot find an elegant solution that fixes the problem at the root for all cases?

@Sebobo
Copy link
Member

Sebobo commented Aug 11, 2020

Not a full answer but does it already help to check the workspace instead via node.context.workspace.name == 'live' for example?

@skurfuerst
Copy link
Member

skurfuerst commented Sep 10, 2020

Hey everybody,

We've just analyzed the issue a bit further:

  • on the current 5.3 instance (just released), the preview links ALWAYS point to your user workspace; e.g. they point to URLs looking like http://127.0.0.1:8081/neos/preview?node=/sites/neosdemo/features/imagetext@user-admin;language=en_US . This is the same URL which is shown inside the content area.
    • This has the problem that hidden elements are rendered; and elements which ask for node.context.inBackend return TRUE so they are rendered differently than on the live website.
  • on Neos 5.2, the URLs look a bit different: /neos/redirect?node=%2Fsites%2Fneosdemo%2Fnode-4ynv3z6wgi9bc%40live (we have the /neos/redirect indirection; AND this is redirected to live workspace.
    • sometimes, the redirect happens to the user workspace (though we do not know the exact cases here)

I'd say both of the behavior is not correct; or what the user expects.

What do we want to archive?

When clicking the preview, the editor wants to see the current editing state of his personal workspace, as if it were live (even if the workspace is not published).

This means:

  • node.context.inBackend must return FALSE.
  • hidden nodes must not be rendered, EXCEPT if the page itself is hidden, it should be rendered.

Suggested behavior change

  • we use different URLs for rendering in backend (inside the iFrame) and when clicking the preview link.
  • based on these URLs, we can set different internal state, to archive the above suggested behavior.

@markusguenther @bwaidelich @Sebobo @kitsunet @paavolimbo What do you think about this proposal?

Open Question

For what release do we want to fix this? I'd say 5.3, because we changed the behavior in this area (and would need to build the fix twice otherwise)

All the best,
Sebastian

@skurfuerst
Copy link
Member

(reminder to self: Timebox for the project: 4h)

@Sebobo
Copy link
Member

Sebobo commented Sep 10, 2020

I recently fixed some stuff in that regard, where the preview link did not update after changes were made and the resulting url was broken: neos/neos-ui#2744

The problem was that there are several cases. We need to make clear which mode we want, which dimension, which workspace, lots of stuff.

So a fix should go to the same version, I think 5.2 possibly. But the preview link was never fully correct.

@paavo
Copy link
Contributor

paavo commented Sep 10, 2020

What do you think about this proposal?
Sounds like a good solution to me 👍

Additional Brain-Dump related to node.context.inBackend
Im mostly using 'node.context.inBackend' && 'node.context.currentRenderingMode.edit' to check for "backend-related" things. With checking for currentRenderingMode.edit, backend messages are not visible in the Backend Preview Central.

@bwaidelich
Copy link
Member

on the current 5.3 instance (just released), the preview links ALWAYS point to your user workspace

For me the preview link points to neos/redirect?... (redirecting to the live version) unless the current documentNode has just been created and published.
After a browser refresh it points to neos/redirect?.. again.

(this doesn't make sense either of course, but I can't reproduce what you described)

In any case I support the idea of introducing yet another route for the actual preview (naming that new route "preview" wasn't a good idea of mine – I was thinking of the editor preview in the iFrame)

@bwaidelich
Copy link
Member

I think the following solution would make sense:

  1. Change the current "preview" route to s.th. like neos/edit?... and only use it in the iFrame
  2. Let the preview link point to a new route neos/preview?.. that
    a) redirects to the proper live URL if the current document node is fully published or otherwise
    b) stays neos/preview?... and renders the preview as described above
  3. Get rid of the neos/redirect route

Alternative to 2b the neos/preview?... route could always trigger a redirect (either to /the/live-node.html or to some other preview route) – I'm not sure about what is better yet.

One advantage of that approach would be that the preview link URL would be fixed (no need to update it on the client side when changes were published/discarded)

@skurfuerst
Copy link
Member

@bwaidelich totally makes sense :)

@skurfuerst
Copy link
Member

FYI: I plan to work on this on Monday // fix it.

@bwaidelich
Copy link
Member

Alternative to 2b the neos/preview?... route could always trigger a redirect (either to /the/live-node.html or to some other preview route) – I'm not sure about what is better yet.

After talking to @skurfuerst we realized that it has to be a separate route because we need to stay in the "preview" mode even if you navigate through the page (otherwise you'd be "caught" in live mode once you navigate to a node without unpublished changes).

Ultimately we'll probably need two buttons (or a split button or whatever makes sense UX wise). One to open the current page in the live mode (if the node exists in live) and one to get a preview of the current page.

And the preview mode could get some "PREVIEW" badge, potentially with a link to the live version.

Some more things we discussed (memory aid):

  • To solve the actual problem described above we probably need to extend the ContentCache so that it knows the difference between "live", "edit" and "preview" mode (and creates separate cache segmentations for those)
  • The LinkingService needs to set the target action according to the mode
  • (not exactly related) We want to rename LinkingService to NodeUriBuilder

This probably sounds like more work than it (hopefully) is. But it will contain profound changes so it can only be implemented with the next major release (due in December).
We should still fix the inconsistent behavior of the preview link – IMO it would be OK if it always redirects to the live version for now

@paavo
Copy link
Contributor

paavo commented Sep 10, 2020

IMO it would be OK if it always redirects to the live version for now

Yes, sounds like a good "temporary solution" as this is what the user (our users) expect.

@klfman
Copy link
Author

klfman commented Sep 10, 2020

IMO it would be OK if it always redirects to the live version for now

Yep, that seems like a valid temporary solution! Okay with me and I like your long term solution for the next major 👍

Thanks!

@DrillSergeant
Copy link
Contributor

IMO it would be OK if it always redirects to the live version for now
That would make our editors happy (and me too). Thanks for taking care of this.

The further plans (split button to choose live/preview of own workspace (without hidden content nodes)) sounds like great plan, looking forward to that.

@bwaidelich
Copy link
Member

bwaidelich commented Sep 11, 2020

OK, I dug a bit deeper and implemented a first draft:
image

...only to realize that we already have in fact a preview mode:
image

So, this would be my new favorite solution:

Short term (bugfix)

The following three lines actually led to the bug, since the UI replaces the preview link target once the current node is published: https://github.com/neos/neos-ui/blob/5.3.0/packages/neos-ui-sagas/src/Publish/index.js#L24-L27

For a started those lines could just be removed. But that would mean that the user has to re-navigate to a newly generated document node (or refresh the browser) after it has been published because otherwise the preview link will stay inactive.

The "invalid" URL is generated here: https://github.com/neos/neos-ui/blob/5.3.0/Classes/Fusion/Helper/NodeInfoHelper.php#L189 and put into the AJAX feedback.
This should instead be a redirect link to the node in live workspace.

Those lines can be removed anyways, since changing the uriPathSegment won't affect the "redirect" URI
https://github.com/neos/neos-ui/blob/5.3.0/packages/neos-ui-sagas/src/UI/Inspector/index.js#L155-L158

Long term

  • Remove the notion "preview" from the PreviewLinkButton and URLs, since this is quite misleading
  • Tweak the default preview mode so that it is more visible (I'll create a separate ticket for that)

@DrillSergeant
Copy link
Contributor

I agree, the preview mode is made for that (viewing the unpublished changes before they are really published). I am not sure if any editor would switch the display-mode in Neos to a preview-mode because switching browser tabs (or windows) is faster and imho also more convenient. Most of the time they anyway want to see the live-result and publish all the time.

For scenarios where editors do lots of editing without publishing, the preview-mode (inside the Neos backend) maybe ok but still it feels more hassle to switch backend-modes between editing content and previewing it (compared to browser tabs).

Currently almost all Neos editors I know work like this:

  • edit in in-place-mode in one browser tab
  • publish quite often
  • after publishing check the result via the button "Show preview" in another browser tab (or even window)

For the link-title (tooltip) which is currently "Show preview" I suggest "Show public version" or "Show live version":
image

@paavo
Copy link
Contributor

paavo commented Sep 14, 2020

Currently almost all Neos editors I know work like this:

  • edit in in-place-mode in one browser tab
  • publish quite often
  • after publishing check the result via the button "Show preview" in another browser tab (or even window)

I totally agree with @DrillSergeant
Most Editors "i know" work exactly like this.

Thanks for your work @bwaidelich 👍

@klfman
Copy link
Author

klfman commented Sep 14, 2020

I'm happy to see this issue progress so dynamically! :) ❤️ Thanks for all your efforts!

In my head this workflow makes sense:

  1. Edit in-place-mode in one browser tab in the Neos Backend
  2. Clicking on the preview button (or switch to already opened tab with preview mode open) -> Control that none of my changes looks bad before publishing
  3. Publish if I'm satisfied

Thus I'm not a fan of a split button, one to "live" and one to "preview", it introduces yet another variant to something that in my point of view does not have a common use case. I cannot imagine explaining the value of this to my customers. If they want to see the live site, they would open the public url and navigate there. That would be a very clear separation of concerns in my head. Live means taking the way the customers do surfing to the live site (easy to grasp concept, just do it like they do and you get what they see ;) ) and anything from Neos is edit/preview-related. As Neos follows a WYSIWYG idea, that would mean "I see and can check/control what I get for my users before publishing" -> this gives control to even more insecure editors before they commit to their edits and hundrets of customers see what they've been working on.

Consequently, the route neos/preview?.. should imho never redirect to the live state (e.g. change the URL to the live site if no edits are detected) because if the editor opens it, edits and then refreshes they would probably be confused that they do not see their edits. In a "clean" publish state the preview url should render the same content as the live url but still have a different URL. But this seems to be already addressed in the concept, because you were discussing about needing to stay in the "preview" mode even if the editors navigates through the page.

@bwaidelich bwaidelich self-assigned this Sep 14, 2020
@bwaidelich bwaidelich moved this from Backlog to In progress in Neos & Flow 7.0 Release Board Sep 14, 2020
bwaidelich added a commit to neos/neos-ui that referenced this issue Sep 14, 2020
With this change the `PreviewButton` of the UI never links to a preview
of the node in the personal user workspace.
Instead it:

* Is disbled if the node is hidden/missing in the base workspace
* Points to a `/neos/redirect?<node-in-base-workspace>` URL that redirects to the current node
  in its base workspace

For the "usual editing case" (personal workspace with target == `live`)
that means, that clicking the button will lead to the version in the live
workspace (i.e. `/the/final/url.html`).

For shared workspaces the redirect will point to a preview URL for that
workspace (i.e. `/neos/preview?node=<node-in-shared-workspace>`)

Fixes: neos/neos-development-collection#3019
@bwaidelich bwaidelich removed their assignment Sep 14, 2020
@bwaidelich
Copy link
Member

bwaidelich commented Sep 15, 2020

Thanks again for all your feedback.

There has been some confusion (partly because I got things wrong), but I worked on a PR that fixes the described bug (I hope): neos/neos-ui#2777

To recap the current behavior (with the fix applied):

  • The preview button opens a new tab that redirects to the currently selected node in its base workspace
    • For the "regular scenario" (personal workspace with target live) that means that it redirects to the final live URL (e.g. /some-node.html)
    • When working on a shared workspace, the redirect will point to a preview of that workspace (e.g. /neos/preview?node=<node-in-shared-workspace>). This allows for navigating/sharing that workspace before it is published to live (an authenticated Neos user is required for that ofc)
  • Since it leads to a preview of the base workspace, unpublished changes of the user won't appear in the preview (for that the preview modes could be used)

The bug itself led to the redirect pointing to the edit mode of the personal user workspace in some cases (e.g. when changes where published and the browser wasn't reloaded, the preview URL was updated to point to the wrong target)

Note: I concentrated on fixing the current behavior. We could discuss the general usefulness of the whole behavior. But that should belong to a separate issue and be synced with the UX guild!

bwaidelich added a commit to neos/neos-ui that referenced this issue Sep 15, 2020
With this change the `PreviewButton` of the UI never links to a preview
of the node in the personal user workspace.
Instead it:

* Is disabled if the node is hidden/missing in the base workspace
* Points to a `/neos/redirect?<node-in-base-workspace>` URL that redirects to the current node
  in its base workspace

For the "usual editing case" (personal workspace with target == `live`)
that means, that clicking the button will lead to the version in the live
workspace (i.e. `/the/final/url.html`).

For shared workspaces the redirect will point to a preview URL for that
workspace (i.e. `/neos/preview?node=<node-in-shared-workspace>`)

Fixes: neos/neos-development-collection#3019
@bwaidelich
Copy link
Member

aaand finally: neos/neos-ui#2781 includes some fixes @skurfuerst came up with and is created against the lowest affected branch (5.1)

@skurfuerst
Copy link
Member

Hey @klfman @DrillSergeant @paavo ,

as already explained by @bwaidelich , what we have fixed here is not 100% what I originally intended to change in the original proposal. However, we found numerous code reasons why the "preview" mode as I imagined it originally was error-prone and would likely lead to more subtle bugs, not less.

Additionally, we felt that duplicating functionality (i.e. "preview modes" vs "show base workspace in new tab") was not the right direction to take without further thought.

Thus, we now fixed the link to always point to the base workspace version (i.e. live or a review workspace); and we suggest to use the condition ${node.context.inBackend && node.context.currentRenderingMode.edit} (as suggested by @paavo) in custom Fusion code for distinguishing between BE and FE.

That leaves one difficult to solve case still open: The preview mode does not hide hidden nodes. This is kinda hard to solve, but I just tried a Fusion workaround which seems to work well:


// For preview modes, we do not want to show hidden nodes.
//
// 1. Thus, we want to switch the node context to `invisibleContentShown=FALSE`.
// 2a. If a child node is hidden, it is not rendered (as expected).
// 2b. If a document node is hidden, this is more difficult:
//     - It can happen that `node` and `documentNode` is NULL, which e.g. crashes @cache.entryTags for instance.
//     - Thus, we need to do a bit of a workaround (see 3.)
// 3. We copy "root" to "rootPreviewModeWithoutHiddenContent", with the changed `node`.
//     - We disable the cache.
//     - We change the node / documentNode's context to `invisibleContentShown=FALSE`.
// 4. In case `node` is null, we render an error page that the document node is hidden.
rootPreviewModeWithoutHiddenContent < root
rootPreviewModeWithoutHiddenContent {
  @cache.mode = 'embed'
  @context {
    documentNode = ${documentNode.context.inBackend && documentNode.context.currentRenderingMode.preview ? q(documentNode).context({"invisibleContentShown": false}).get(0) : documentNode}
    node = ${node.context.inBackend && node.context.currentRenderingMode.preview ? q(node).context({"invisibleContentShown": false}).get(0) : node}
    origContext = ${node.context}
  }

  errorDisplayWhenNodeIsHidden {
    @position = 'start'
    condition = ${!node && origContext.inBackend && origContext.currentRenderingMode.preview}
    renderer = afx`
      <html>
      <head>
        <link rel="stylesheet" @children="attributes.href">
          <Neos.Fusion:ResourceUri path="resource://Neos.Neos/Public/Styles/Error.css"/>
        </link>
      </head>
      <body class="neos">
      <div class="neos-error-screen">
        <div class="neos-message-header">
          <div class="neos-message-icon">
            <i class="fas fa-warning-sign"></i>
          </div>
          <h1>Page is hidden, thus the preview cannot be shown</h1>
        </div>
        <p>Please unhide the page, then we can show the preview accordingly.</p>
      </div>
      </body>
      </html>
    `
  }
}

root.renderingPreviewMode {
  @position = 'start'
  condition = ${node && node.context.inBackend && node.context.currentRenderingMode.preview}
  renderPath = '/rootPreviewModeWithoutHiddenContent'
}

You can try out this, and see if that works for you :) If it does work well, we can document this on docs.neos.io.

All the best,
Sebastian

@DrillSergeant
Copy link
Contributor

@bwaidelich and @skurfuerst thank you guys for fixing the preview-button. Looking forward for the release to roll it out to our customers.

About the hidden nodes in preview-mode: This was not an issue for our editors so far. If this comes up, I will dig into it and try you code example.

markusguenther pushed a commit to neos/neos-ui that referenced this issue Sep 17, 2020
…#2781)

* BUGFIX: Prevent preview button to link to the personal user workspace

With this change the `PreviewButton` of the UI never links to a preview
of the node in the personal user workspace.
Instead it:

* Is disabled if the node is hidden/missing in the base workspace
* Points to a `/neos/redirect?<node-in-base-workspace>` URL that redirects to the current node
  in its base workspace

For the "usual editing case" (personal workspace with target == `live`)
that means, that clicking the button will lead to the version in the live
workspace (i.e. `/the/final/url.html`).

For shared workspaces the redirect will point to a preview URL for that
workspace (i.e. `/neos/preview?node=<node-in-shared-workspace>`)

Fixes: neos/neos-development-collection#3019

* Remove obsolete workspace check from NodeInfoHelper::createRedirectToNode()

* Respect the published node when updating the previewUrl

In order to prevent the preview button to point to a
wrong node after publishing all pending changes
@markusguenther
Copy link
Member

I just tested and merged the change but I want to thank all participants here for the very fruitful discussion. Would be so cool have this on more issues 😄

🥇 for all of you :)

Neos & Flow 7.0 Release Board automation moved this from In progress to Done Sep 18, 2020
@lorenzulrich
Copy link
Contributor

@paavo Does isInBackend = ${node.context.inBackend && node.context.currentRenderingMode.edit} return false for you in a page like https://foobar.test/neos/preview?node%5B__contextNodePath%5D=%2Fsites%2Fcorporate%2Fnode-iiotd955c20l5%40freigabe-km-3p0o1%3Blanguage%3Dde?
In my case, this still returns true.

@paavo
Copy link
Contributor

paavo commented May 6, 2022

Im using a Helper like this @lorenzulrich what returns false in preview Mode ☝️

prototype(Vendor.Base:Helper.Backend) < prototype(Neos.Fusion:Component) {
    renderer = ${node.context.inBackend && node.context.currentRenderingMode.edit ? true : false}
}

@lorenzulrich
Copy link
Contributor

This is very strange. Are we talking about "Preview Central" or a preview link?

When I dump getCurrentRenderingMode when opening a preview link, I get:

image

@paavo
Copy link
Contributor

paavo commented May 6, 2022

I was talking about "Preview Central" @lorenzulrich 😆

This i get in "Neos-Backend" Preview Central:
image

This i get when i preview a hidden Page (in Frontend)
image

@lorenzulrich
Copy link
Contributor

lorenzulrich commented May 6, 2022

@paavo Thanks, but your preview is a preview of the LIVE workspace, not a restricted workspace. Maybe I need to post steps to reproduce.

@paavo
Copy link
Contributor

paavo commented May 6, 2022

restricted workspace? the ones published by flownative/workspace-preview?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

8 participants