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

fix: prevent flush changes when synchronizing properties #15583

Merged
merged 1 commit into from
Jan 20, 2023

Conversation

mcollovati
Copy link
Collaborator

@mcollovati mcollovati commented Dec 29, 2022

DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615

@github-actions
Copy link

github-actions bot commented Dec 29, 2022

Test Results

   937 files  ±0     937 suites  ±0   54m 57s ⏱️ - 1m 48s
5 871 tests +4  5 836 ✔️ +4  35 💤 ±0  0 ±0 
6 126 runs  +5  6 084 ✔️ +5  42 💤 ±0  0 ±0 

Results for commit 4673fff. ± Comparison against base commit c8e1f3d.

♻️ This comment has been updated with latest results.

@vursen
Copy link
Contributor

vursen commented Jan 3, 2023

Shall we add some tests?

@mcollovati
Copy link
Collaborator Author

Shall we add some tests?

I'll try to see if I'm able to create at least a GWT test

@mcollovati mcollovati force-pushed the fix/flushing_on_dom_events branch 2 times, most recently from 279a304 to c8458a8 Compare January 10, 2023 08:22
DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615
@sonarcloud
Copy link

sonarcloud bot commented Jan 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@knoobie
Copy link
Contributor

knoobie commented Jan 17, 2023

Shouldn't this fix be prioritized because the feature it fixes was also backported and is released in 23.3 and below?

@mcollovati
Copy link
Collaborator Author

@vursen do you have time to take another look into this one?

ping also @manolo, do you mind looking at GWT tests?

@vursen
Copy link
Contributor

vursen commented Jan 18, 2023

@mcollovati The solution looks good to me. As for tests, I would leave that to @manolo to review as I'm not familiar with GWT.

@knoobie
Copy link
Contributor

knoobie commented Jan 19, 2023

Thanks guys 🙇‍♂️

@mcollovati mcollovati merged commit 6adb7e9 into master Jan 20, 2023
@mcollovati mcollovati deleted the fix/flushing_on_dom_events branch January 20, 2023 06:28
vaadin-bot pushed a commit that referenced this pull request Jan 20, 2023
DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615
vaadin-bot pushed a commit that referenced this pull request Jan 20, 2023
DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615
vaadin-bot pushed a commit that referenced this pull request Jan 20, 2023
DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615
vaadin-bot pushed a commit that referenced this pull request Jan 20, 2023
DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615
vaadin-bot added a commit that referenced this pull request Jan 20, 2023
…5722)

DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615

Co-authored-by: Marco Collovati <marco@vaadin.com>
vaadin-bot added a commit that referenced this pull request Jan 20, 2023
…5723)

DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615

Co-authored-by: Marco Collovati <marco@vaadin.com>
vaadin-bot added a commit that referenced this pull request Jan 20, 2023
…5725)

DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615

Co-authored-by: Marco Collovati <marco@vaadin.com>
vaadin-bot added a commit that referenced this pull request Jan 20, 2023
…5724)

DOM events related to property synchronization must not flush changes
to avoid wrong execution order of events on server side.
For example, an 'has-input-value-changed' event may occur before the
actual value of a field has been changed and a flush will trigger
listeners with an old value.

Fixes #15615

Co-authored-by: Marco Collovati <marco@vaadin.com>
@vaadin-bot
Copy link
Collaborator

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

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

Successfully merging this pull request may close these issues.

Regression: property synchronization events cause a value flush
5 participants