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

UI dirty after changes are written on the response causes push invocation #14887

Closed
mcollovati opened this issue Oct 19, 2022 · 6 comments · Fixed by #15517
Closed

UI dirty after changes are written on the response causes push invocation #14887

mcollovati opened this issue Oct 19, 2022 · 6 comments · Fixed by #15517
Assignees
@mcollovati
Labels
bug Impact: High prerelease version for Vaadin 24.0.0 Released with Vaadin 14.9.4 Released with Vaadin 22.0.26 Released with Vaadin 23.2.12 Released with Vaadin 23.3.2 Released with Vaadin 24.0.0.alpha7 Severity: Minor
Projects
OLD Vaadin Flow bugs & maintenance (V...

Comments

@mcollovati
Copy link
Collaborator

Description of the bug

It seems like that in some situations after UidlRequestHandler has written the changes on the response, the UI is still considered dirty, causing push to be invoked when VaadinSession is unlocked.

For example, when opening a Vaadin application on the browser, there's a first UIDL request from the client that is processed and written to the response (UidlRequestHandler.synchronizedHandleRequest -> commitJsonResponse) and the client issues a new UIDL request, that is pending.
The first request continues with the UI still dirty (UIInternals.isDirty()) and push() is invoked
The push message contains no changes, and it is processed by the client.
After that, the pending request continues, but this time after UidlRequestHandler processing the UI is not dirty (perhaps because there are no changes).

The same happens for example also when pressing a button with a server-side listener:
UIDL req -> response written -> UI still dirty -> push

In the following screenshot you can see the UIDL request, the push message without changes and the second UIDL request.

image

Expected behavior

After a UIDL request is processed, the UI should not be dirty and no PUSH messages are sent to the client.

Minimal reproducible example

  • Create a new application with Hello World using start.vaadin.com (npx @vaadin/cli init --latest myapp)
  • Put a breakpoint on last line of UidlRequestHandler.synchronizedHandleRequest, after commitJsonResponse()
  • Run the application with debugger attached
  • Open the application in the browser (http://localhost:8080)
  • When the breakpoint hits, check that the UI is still dirty (watch uI.getInternals().isDirty())

Versions

  • Vaadin / Flow version: 23.2.5
  • Java version: 11
@mcollovati
Copy link
Collaborator Author

Observations from @Legioth

StateTree.collectChanges does first collect all dirty nodes while emptying the dirtyNodes collection.
Then it does StateNode.collectChanges on each collected dirty node which for a newly attached node runs NodeFeature::generateChangesFromEmpty which at least in the case of ElementChildrenList with the isRemoveAllCalled flag set does addChange(new ListClearChange<>(this)) which in turn marks the node as dirty again.
I'm not sure if the solution here would be to change the generateChangesFromEmpty logic to not mark the node as dirty or if the dirty nodes should be collected once more after iterating through StateNode.collectChanges.

@mcollovati mcollovati added this to Needs triage in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) via automation Oct 19, 2022
@mcollovati mcollovati moved this from Needs triage to New P2 in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Oct 19, 2022
@mcollovati mcollovati mentioned this issue Nov 20, 2022
Closed
@mshabarov mshabarov mentioned this issue Dec 16, 2022
Closed
@mcollovati mcollovati self-assigned this Dec 16, 2022
mcollovati added a commit that referenced this issue Dec 16, 2022
@mcollovati
fix: collect changes until UI is not dirty
ec3a150 
When UidlWriter collects changes to send to the client it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
@mcollovati mcollovati mentioned this issue Dec 16, 2022
Merged
9 tasks
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from WIP to Closed Dec 19, 2022
mcollovati added a commit that referenced this issue Dec 19, 2022
@mcollovati
fix: collect changes until UI is not dirty (#15517)
ddae1ed 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
mcollovati added a commit that referenced this issue Dec 19, 2022
@mcollovati
fix: collect changes until UI is not dirty (#15517)
55d9497 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
@mcollovati mcollovati mentioned this issue Dec 19, 2022
Merged
mcollovati added a commit that referenced this issue Dec 19, 2022
@mcollovati
fix: collect changes until UI is not dirty (#15517) (#15521)
46a3214 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
vaadin-bot pushed a commit that referenced this issue Dec 19, 2022
@mcollovati @vaadin-bot
fix: collect changes until UI is not dirty (#15517) (#15521)
c1e0b88 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
mcollovati added a commit that referenced this issue Dec 19, 2022
@mcollovati
fix: collect changes until UI is not dirty (#15517) (#15521)
01db434 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
@mcollovati mcollovati mentioned this issue Dec 19, 2022
Merged
vaadin-bot added a commit that referenced this issue Dec 19, 2022
@vaadin-bot @mcollovati
fix: collect changes until UI is not dirty (#15517) (#15521) (#15524)
e3163f0 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887

Co-authored-by: Marco Collovati <marco@vaadin.com>
mcollovati added a commit that referenced this issue Dec 19, 2022
@mcollovati
fix: collect changes until UI is not dirty (#15517) (#15521)
15b8069 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
@mcollovati mcollovati mentioned this issue Dec 19, 2022
Merged
mcollovati added a commit that referenced this issue Dec 20, 2022
@mcollovati
fix: collect changes until UI is not dirty (#15517) (#15521) (#15527)
1ad9e90 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
mcollovati added a commit that referenced this issue Dec 20, 2022
@mcollovati
fix: collect changes until UI is not dirty (#15517) (#15521) (#15525)
190cdd2 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
MarcinVaadin pushed a commit that referenced this issue Dec 21, 2022
@mcollovati @MarcinVaadin
fix: collect changes until UI is not dirty (#15517)
d528966 
When UidlWriter collects changes to send to the client, it may happen
that UI is still dirty because features may enqueue additional changes.
As a side effect, if UI is still dirty after UIDL response is written,
a useless PUSH action is performed.
This change performs changes collection until the UI is no more dirty.

Fixes #14887
@TatuLund TatuLund mentioned this issue Dec 23, 2022
Closed
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 14.9.4.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 22.0.26.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.2.12.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.3.2.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.0.0.alpha7 and is also targeting the upcoming stable 24.0.0 version.

@Artur- Artur- mentioned this issue Jan 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mcollovati mcollovati

Labels
bug Impact: High prerelease version for Vaadin 24.0.0 Released with Vaadin 14.9.4 Released with Vaadin 22.0.26 Released with Vaadin 23.2.12 Released with Vaadin 23.3.2 Released with Vaadin 24.0.0.alpha7 Severity: Minor
Projects
OLD Vaadin Flow bugs & maintenance (V...
Closed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: collect changes until UI is not dirty vaadin/flow
2 participants
@mcollovati @vaadin-bot