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

Grid addComponentColumn is Broken in Latest Google Chrome Version #5025

Closed
dwlabcube opened this issue Feb 6, 2019 · 23 comments · Fixed by #5060
Closed

Grid addComponentColumn is Broken in Latest Google Chrome Version #5025

dwlabcube opened this issue Feb 6, 2019 · 23 comments · Fixed by #5060
Assignees
Labels
Milestone

Comments

@dwlabcube
Copy link

When i run the latest bakery-app and change the following two Lines in AbstractBakeryCrudView the /users view is broken and the edit-Button often only come after a Ctrl+F5 reload of the page.

It only happens since the release of Google Chrome Version 72.0.3626.81

Code: grid.setDataProvider(dataProvider);

setupGrid(grid);

//Crud.addEditColumn(grid); --> This Line is the original which uses a TemplateRenderer

grid.addComponentColumn(e -> new Button("edit")); // This line is the new Line and it uses a componentColumn

code
buttons_missing
buttons_visible

@heruan
Copy link
Member

heruan commented Feb 6, 2019

I'm using 13.0.0.alpha4 and I can confirm that component columns don't always work, i.e. randomly appear as empty. This could happen at first page load, but also during in-app navigation; usually a page refresh fixes it, but in a couple of cases I had to refresh twice.

@dinumihnea
Copy link

Also in 12.0 (all) versions of Vaadin the bug is present, after Chrome update!

@silvan-lincan
Copy link

I can confirm the same issue with TreeGrid-Editor in both Vaadin 12.0.5 and 13.0.0.beta1.

@nemojmenervirat
Copy link

for someone who is using polymer template, you can delete and insert this box-sizing rule and flow-component-renderer will always work in latest chrome (Version 72.0.3626.96 (Official Build) (64-bit)):

afterServerUpdate(){
                var styleSheet = this.shadowRoot.styleSheets[0];
                styleSheet.deleteRule(1);
                styleSheet.insertRule('*,::after,::before { box-sizing:border-box }', 1);
}

@HaydenDekker
Copy link

I've noticed that ui events on other elements on the same page can cause the to render. For example, I have a combobox above the grid and selecting the drop down causes the grid custom components to partially render. The page correctly loads the HTML it's just not rendered.

@dinumihnea
Copy link

for someone who is using polymer template, you can delete and insert this box-sizing rule and flow-component-renderer will always work in latest chrome (Version 72.0.3626.96 (Official Build) (64-bit)):

afterServerUpdate(){
                var styleSheet = this.shadowRoot.styleSheets[0];
                styleSheet.deleteRule(1);
                styleSheet.insertRule('*,::after,::before { box-sizing:border-box }', 1);
}

is possible to do this using external CSS?

@pleku
Copy link
Contributor

pleku commented Feb 12, 2019

Something in Chrome 72 has changed and broken this, my guess is that it is https://chromium-review.googlesource.com/c/chromium/src/+/1350441

I've just tested this with https://www.google.com/chrome/canary/ and it is working again - I'm not able to reproduce the issue.
For Dev and Beta channels, it is still broken.

@pleku pleku self-assigned this Feb 12, 2019
pleku added a commit that referenced this issue Feb 12, 2019
@pleku
Copy link
Contributor

pleku commented Feb 12, 2019

Just guessing, but the fix might have been https://chromium-review.googlesource.com/c/chromium/src/+/1444151 or https://chromium.googlesource.com/chromium/src/+/3e610731bc6f24b929249d336835a865a791a69b

Currently those fixes are landing to Chrome 74, and we have no idea if those will be picked to 72 or 73. So we are looking at whether we can somehow trigger something in flow-component-renderer that will fix address the issue for now.

@Legioth
Copy link
Member

Legioth commented Feb 12, 2019

Seems like this was accidentally closed by typo in Closes definition of unrelated PR

@Legioth Legioth reopened this Feb 12, 2019
@heruan
Copy link
Member

heruan commented Feb 12, 2019

Thank you @pleku for the investigation and findings! This is indeed a serious issue as it suddenly started to affect users of apps already in production with V12.

@erszoke
Copy link

erszoke commented Feb 13, 2019

I agree with @heruan about the seriousity, our company has three affected applications in production.

@heruan
Copy link
Member

heruan commented Feb 13, 2019

Question for the team: which is the strategy here? Being a Chrome bug, I suppose it's not certain it can be fixed on the component's side. Are you in contact with Chrome developers about this? Since it appears the fix to be already there, it's vital they know it affects existing products and it should be backported to the current Chrome version.

@pleku
Copy link
Contributor

pleku commented Feb 13, 2019

Question for the team: which is the strategy here? Being a Chrome bug, I suppose it's not certain it can be fixed on the component's side. Are you in contact with Chrome developers about this? Since it appears the fix to be already there, it's vital they know it affects existing products and it should be backported to the current Chrome version.

We have a working fix for Grid. But since the issue might also be in other components that use flow-component-renderer, we want to make the fix so that it can also work with any component without going in and changing and releasing Flow again for this (flow-data contains component renderer).

The fix will only be applied for Chrome versions >71 and we will test with newer Chrome versions so that we can then limit the workaround for only effected versions. There will be a small performance hit from the workaround. As I said, in Canary channel (version 74) this is already fixed, but not in beta/dev channels (version 73).

The bug itself is quite nasty regression for web components using shadow-dom, so it might be that we can pass this information to Chrome team using our contacts. The (presumably) related bug ticket itself seems to be private, I cannot open it.

@jetdario
Copy link

jetdario commented Mar 5, 2019

Hi guys, was the issue has already been fix? Im having the same problem now.

@denis-anisimov
Copy link
Contributor

It has been fixed.
Please check which version you use.
It might be that in your version it's not fixed yet.

@Legioth
Copy link
Member

Legioth commented Mar 5, 2019

To clarify, the fix has been included in Vaadin versions 10.0.11, 12.0.7, and 13.0.0.beta2.

@jetdario
Copy link

jetdario commented Mar 5, 2019

Thanks denis and Legioth.. updated my version it works.

@nemanjakmno
Copy link

First row in grid with only one component column not visible in chrome 72+ . After page resize or data provider refresh it appears.

image

@Legioth
Copy link
Member

Legioth commented Mar 8, 2019

@kolle1986 What version of Vaadin and/or Flow are you using?

@nemanjakmno
Copy link

nemanjakmno commented Mar 8, 2019

@Legioth 13.0.0 - the latest version

@Legioth
Copy link
Member

Legioth commented Mar 11, 2019

@kolle1986 In that case, please open a new issue with instructions on how to reproduce. We won't reopen this issue since that would make it more difficult keep track of which changes have been included in which version.

@dogInTheWok
Copy link

The same here: 13.0.1 with Chromium 72.0.3626.121 on Ubuntu 18.04.2 LTS we still observe the issue, i.e. custom components like images or buttons are not loaded in tables. After Ctrl + F5 it is rendered correctly. Scrolling also fixes the issue. SImple refresh (F5) does not work though. In Firefox everything is rendered correctly.

@pleku
Copy link
Contributor

pleku commented Mar 13, 2019

@dogInTheWok please create a new issue with the instructions (or even better, code) to reproduce.

manolo pushed a commit that referenced this issue Mar 19, 2019
* Send service URL to the client side for Web Components UI (#5031)

Fixes #5044
Fixes #5027

* Remove dependency from data to html-components (#5050)

Fixes #5025

* Clarify StreamResource JavaDoc (#4981)

* 1.5-SNAPSHOT (#5055)

* Fix flow-component-renderer for Chrome 72 (#5060)

Fixes #5025, vaadin/vaadin-grid-flow#502

* Make the java doc return say the correct thing. (#5063)

* Shortcuts: change click shortcut's default behavior (#5058) (#5065)

* Deprecated @VaadinServletConfiguration (#5052)

* Update chrome driver version (#5020)

* Update byte buddy version (#5026)

* Update byte buddy version

Fixes #4956

* Fix version id in drivers (#5083)

* Deprecate getBodyAttributes and replace it with getHtmlAttributes (#5019) Fixes #4765

* Add IT test for push in embedded web components (#5059) Fixes #5006

* Add m2e lifecycle mapping for maven-antrun-plugin (#5087)

* Add m2e lifecycle mapping for maven-antrun-plugin

* Catch Throwable during maps sync handling (#5077)

* Shortcuts no longer work, when element is disabled (#5099)

* Shortcuts no longer work, when element is disabled

* Fix getChildCount to always use filter  (#5101) (Fixes vaadin-grid-flow/issues/464)

* Fix invalid null check (#5132)

* Javadoc - Add a component already added to a parent. (#5137)

* Document the fact that a component with a parent will be added to a new parent and removed from the previous one.

* Added Thread.sleeps to make sure Enter key is registered (#5139)

* flow-component-renderer: set focus-target attribute to the first focusable node (#5142)

Fixes #4191

Defining focus-target is necessary for the grid cell delegating focus to the first focusable component within it when Enter key is used (tests available from another PR on vaadin-grid-flow repo)

* Add some configuration for surefire plugin (#5144)

* Add some configuration for surefire plugin

Due to issues with tests run under Windows 10 we need to
add the trimStackTrace and reuseForks configurations also
to the surefire plugin

* Also update the surefire and failsafe plugins.

* Register push static web resources (#5112)

* Avoid for..of loop in renderer (#5171)

* Add DOM listeners before attaching the element (#5195)

Fixes #5152

* Add a generic mechanism for passing values back to the server (#5093)

This is an enabler for #1724 and #4925.

Fixes #4927

* Register superclass in case of route target collision (#5218)

Fixes #5114

* Introduce ValueChangeMode#LAZY, and #TIMEOUT (#4945)

Introduces ValueChangeMode#LAZY, and #TIMEOUT for delayed value synchronization to the server and makes Input implement HasValueChangeMode.

* Check the function presence before calling it (#5208)

Check the function presence before calling it and take care about children property as well.

Fixes #5206

* enable TrackMessageSizeInterceptor to deal with multiple messages in same response

* test for _trackMessageSize responsible to handle multiple messages coming from a single response

* refactor imports

* refactor TrackMessageSize test

* using IOUtils to read from InputStream

* Test for Long Polling when multiple threads run within a short period

* Enhanced code

* refactor getFileContent to use full path to retrieve vaadinPush.js

* Correct javadocs for DomEvent (#5215)

Fixes #5211

* Update jsoup version (#5253)

Fixes #4127

* Avoid indeterministic behavior in tests. (#5283)

Set locale explicitly in the tests.

Fixes #3500

* Handle empty attribute values properly in HTML element (#5255)

Fixes #5220

* Update chrome version in tests (#5295)

* Remove redundant words form javadocs (#5294)

Fixes #2770

* Update chromedriver version (#5297)

* Exporter-based embeddable web component impl. (#5260)

WebComponent is currently named IWebComponent to avoid naming collisions with
the existing classes - this will be fixed once the functionality has been
transferred over to the new components.

* Merge branch 'master' into issues/merge_from_master

# Conflicts:
#	build-tools/pom.xml
#	flow-bom/pom.xml
#	flow-client/pom.xml
#	flow-components-parent/flow-code-generator-api/pom.xml
#	flow-components-parent/flow-code-generator/pom.xml
#	flow-components-parent/flow-component-demo-helpers/pom.xml
#	flow-components-parent/flow-generated-components/pom.xml
#	flow-components-parent/flow-webcomponent-api-analyzer/pom.xml
#	flow-components-parent/pom.xml
#	flow-data/pom.xml
#	flow-data/src/test/java/com/vaadin/flow/data/binder/testcomponents/TestLabel.java
#	flow-html-components-testbench/pom.xml
#	flow-html-components/pom.xml
#	flow-maven-plugin/pom.xml
#	flow-osgi/pom.xml
#	flow-push/pom.xml
#	flow-server-production-mode/pom.xml
#	flow-server/pom.xml
#	flow-server/src/main/java/com/vaadin/flow/component/WebComponent.java
#	flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentMethod.java
#	flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentProvider.java
#	flow-server/src/main/java/com/vaadin/flow/server/webcomponent/WebComponentGenerator.java
#	flow-server/src/test/java/com/vaadin/flow/internal/nodefeature/ReturnChannelMapTest.java
#	flow-server/src/test/java/com/vaadin/flow/server/BootstrapHandlerTest.java
#	flow-server/src/test/java/com/vaadin/flow/server/communication/WebComponentProviderTest.java
#	flow-server/src/test/java/com/vaadin/flow/server/communication/rpc/ReturnChannelHandlerTest.java
#	flow-server/src/test/java/com/vaadin/flow/server/webcomponent/WebComponentGeneratorTest.java
#	flow-test-generic/pom.xml
#	flow-test-util/pom.xml
#	flow-tests/pom.xml
#	flow-tests/servlet-containers/felix-jetty/pom.xml
#	flow-tests/servlet-containers/pom.xml
#	flow-tests/servlet-containers/tomcat85/pom.xml
#	flow-tests/servlet-containers/tomcat9/pom.xml
#	flow-tests/test-common/pom.xml
#	flow-tests/test-dev-mode/pom.xml
#	flow-tests/test-frontend-production-custom-context/pom.xml
#	flow-tests/test-lumo-theme/pom.xml
#	flow-tests/test-material-theme/pom.xml
#	flow-tests/test-memory-leaks/pom.xml
#	flow-tests/test-pwa/pom.xml
#	flow-tests/test-resources/pom.xml
#	flow-tests/test-root-context/pom.xml
#	flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/DomListenerOnAttachView.java
#	flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/ReturnChannelView.java
#	flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/ReturnChannelIT.java
#	flow-tests/test-root-ui-context/pom.xml
#	flow-tests/test-router-custom-context/pom.xml
#	flow-tests/test-scalability/pom.xml
#	flow-tests/test-servlet/pom.xml
#	flow-tests/test-subcontext/pom.xml
#	flow-tests/test-themes/pom.xml
#	flow-tests/test-web-components/pom.xml
#	flow-tests/test-web-components/src/main/java/com/vaadin/flow/webcomponent/ClientSelect.java
#	flow-tests/test-web-components/src/main/java/com/vaadin/flow/webcomponent/UpdateServerSideWebComponent.java
#	flow-tests/test-web-components/src/main/webapp/push.html
#	flow-tests/test-web-components/src/test/java/com/vaadin/flow/webcomponent/UpdateServerSideWebComponentIT.java
#	flow-theme-integrations/lumo/pom.xml
#	flow-theme-integrations/material/pom.xml
#	flow-theme-integrations/pom.xml
#	flow/pom.xml
#	pom.xml
manolo pushed a commit that referenced this issue Apr 8, 2019
* Send service URL to the client side for Web Components UI (#5031)

Fixes #5044
Fixes #5027

* Remove dependency from data to html-components (#5050)

Fixes #5025

* Clarify StreamResource JavaDoc (#4981)

* 1.5-SNAPSHOT (#5055)

* Fix flow-component-renderer for Chrome 72 (#5060)

Fixes #5025, vaadin/vaadin-grid-flow#502

* Make the java doc return say the correct thing. (#5063)

* Shortcuts: change click shortcut's default behavior (#5058) (#5065)

* Deprecated @VaadinServletConfiguration (#5052)

* Update chrome driver version (#5020)

* Update byte buddy version (#5026)

* Update byte buddy version

Fixes #4956

* Fix version id in drivers (#5083)

* Deprecate getBodyAttributes and replace it with getHtmlAttributes (#5019) Fixes #4765

* Add IT test for push in embedded web components (#5059) Fixes #5006

* Add m2e lifecycle mapping for maven-antrun-plugin (#5087)

* Add m2e lifecycle mapping for maven-antrun-plugin

* Catch Throwable during maps sync handling (#5077)

* Shortcuts no longer work, when element is disabled (#5099)

* Shortcuts no longer work, when element is disabled

* Fix getChildCount to always use filter  (#5101) (Fixes vaadin-grid-flow/issues/464)

* Fix invalid null check (#5132)

* Javadoc - Add a component already added to a parent. (#5137)

* Document the fact that a component with a parent will be added to a new parent and removed from the previous one.

* Added Thread.sleeps to make sure Enter key is registered (#5139)

* flow-component-renderer: set focus-target attribute to the first focusable node (#5142)

Fixes #4191

Defining focus-target is necessary for the grid cell delegating focus to the first focusable component within it when Enter key is used (tests available from another PR on vaadin-grid-flow repo)

* Add some configuration for surefire plugin (#5144)

* Add some configuration for surefire plugin

Due to issues with tests run under Windows 10 we need to
add the trimStackTrace and reuseForks configurations also
to the surefire plugin

* Also update the surefire and failsafe plugins.

* Register push static web resources (#5112)

* Avoid for..of loop in renderer (#5171)

* Add DOM listeners before attaching the element (#5195)

Fixes #5152

* Add a generic mechanism for passing values back to the server (#5093)

This is an enabler for #1724 and #4925.

Fixes #4927

* Register superclass in case of route target collision (#5218)

Fixes #5114

* Introduce ValueChangeMode#LAZY, and #TIMEOUT (#4945)

Introduces ValueChangeMode#LAZY, and #TIMEOUT for delayed value synchronization to the server and makes Input implement HasValueChangeMode.

* Check the function presence before calling it (#5208)

Check the function presence before calling it and take care about children property as well.

Fixes #5206

* enable TrackMessageSizeInterceptor to deal with multiple messages in same response

* test for _trackMessageSize responsible to handle multiple messages coming from a single response

* refactor imports

* refactor TrackMessageSize test

* using IOUtils to read from InputStream

* Test for Long Polling when multiple threads run within a short period

* Enhanced code

* refactor getFileContent to use full path to retrieve vaadinPush.js

* Correct javadocs for DomEvent (#5215)

Fixes #5211

* Update jsoup version (#5253)

Fixes #4127

* Avoid indeterministic behavior in tests. (#5283)

Set locale explicitly in the tests.

Fixes #3500

* Handle empty attribute values properly in HTML element (#5255)

Fixes #5220

* Update chrome version in tests (#5295)

* Remove redundant words form javadocs (#5294)

Fixes #2770

* Update chromedriver version (#5297)

* Exporter-based embeddable web component impl. (#5260)

WebComponent is currently named IWebComponent to avoid naming collisions with
the existing classes - this will be fixed once the functionality has been
transferred over to the new components.

* Fixed a NPE problem in WebComponentConfigurationRegistry (#5304)

* Implement open and setLocation method to Page. (#4869) (#5166)

* Fix navigate back to main view don't work in FF and Safari . (#5113) (#5291)

* Register a servlet automatically if WCs present (#5331)

Fixes #5070

* EWC: push property updates to client (#5324)

* Apply theme for web component exporters (#5341)

Fix for #4984

* Only make banner the target of pointer events (#5343)

Currently the prompt consists of a full width transparent `#pwa-ip` div which is preventing click in elements behind it, and a `.banner` div which should be clickable

* WebComponent fireEvent implementation (#5340)

Implements #5272

* Fixes #5351 (#5358)

* Detect Push from WebComponentExporter classes (#5369), Fix for #4984

* Make exported web components work on hosts where https is proxied to http (#5374)

* Update config and generated class for rich-text-editor (#4754)

* Merge InstanceConfigurator with exporter (#5376) (Fixes #5332)

Also: Ignore abstract class in exporters discovering logic

* Use "create" semantic for the createComponent method (#5379)

Fixes #4634

* Avoid "web context" in web component module generation (#5391)

* Avoid "web context" in web component module generation

Fixes #5382

* Correct unit test

* Improve JS code

* Extract web component config factory (#5403)

Fixes #5402

* Correct generated template to work properly in production mode (#5421)

* Generate web component module file in the maven plugin to be able to transpile it (#5419)

* Initial preparation for web component module files generation

* Generate web component module content and add it into the bundle for
transpilation

* Generate web component module content and add the generated file into
the bundle file

* Hide common generation method

* Correct typos.

* Merge master into 2.0

Resolve the merge conflicts.

* Remove empty @SInCE javadocs

* Remove unused class

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

Successfully merging a pull request may close this issue.