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: Use feature detection to show the unsupported browser page #15813

Merged
merged 9 commits into from
Feb 16, 2023

Conversation

Artur-
Copy link
Member

@Artur- Artur- commented Feb 2, 2023

Fixes #14366

+ "';window.location.reload();return false;\" href=\"#\">Continue without updating</a> (not recommended)</sub></p>"
+ "</body>\n" + "</html>");
// @formatter:on
IOUtils.copy(getClass().getResourceAsStream("too-old.html"), page,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to allow people to customize this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean in a less fragile way than adding your own src/main/resources/com/vaadin/flow/server/too-old.html ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you say it like this: yes 😅

Copy link
Member Author

@Artur- Artur- Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok now you can use src/main/resources/browser-too-old.html instead :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🙇‍♂️ This is great! Having a way to customize this page is really good. We had requests in the past especially for translation and corporate styling needs. Could be something worth to mention as new feature for v24 - some people are really looking forward to it. Customizing it atm is just hacky

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

Test Results

   946 files  ±  0     946 suites  ±0   57m 38s ⏱️ + 1m 23s
5 913 tests  -   6  5 878 ✔️  -   6  35 💤 ±0  0 ±0 
6 164 runs   - 14  6 122 ✔️  - 14  42 💤 ±0  0 ±0 

Results for commit 22e2ec0. ± Comparison against base commit 770cdda.

♻️ This comment has been updated with latest results.

@mshabarov mshabarov self-requested a review February 6, 2023 12:06
@Artur-
Copy link
Member Author

Artur- commented Feb 7, 2023

Note that if this is backported (it probably should be) then the feature set to detect is different (ResizeObserver should be ok)

@Artur- Artur- requested a review from mcollovati February 16, 2023 07:10
@sonarqubecloud
Copy link

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

Copy link
Collaborator

@mcollovati mcollovati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
I just wonder if we need to mark this as a "breaking change" since we removed a public API.
Perhaps nobody is using the WebBrowser.isTooOldToFunctionProperly() method, but it would be good to mention the removal in the release notes

@Artur-
Copy link
Member Author

Artur- commented Feb 16, 2023

Yes, we could mention that even though usage is likely below 0.1%
It is almost like we could deprecate the whole WebBrowser class and say that you should not do browser detection..

if (!('CSSLayerBlockRule' in window)) {
window.location.search='v-r=oldbrowser';
}
""");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is it a place where we put the checks for used features?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the place yes

@Artur- Artur- merged commit 982d1f2 into main Feb 16, 2023
@Artur- Artur- deleted the too-old-features branch February 16, 2023 08:40
vaadin-bot pushed a commit that referenced this pull request Feb 16, 2023
)

This removes the `WebBrowser. isTooOldToFunctionProperly()` method as it is no longer possible to determine this information based on the user agent.

Fixes #14366
mcollovati pushed a commit that referenced this pull request Feb 16, 2023
) (#15926)

This removes the `WebBrowser. isTooOldToFunctionProperly()` method as it is no longer possible to determine this information based on the user agent.

Fixes #14366

Co-authored-by: Artur <artur@vaadin.com>
Artur- added a commit that referenced this pull request Feb 24, 2023
) (CP: 23.3)

Changed to detect ResizeObserver

This removes the `WebBrowser. isTooOldToFunctionProperly()` method as it is no longer possible to determine this information based on the user agent.

Fixes #14366
Artur- added a commit that referenced this pull request Mar 5, 2023
) (CP: 23.3)

Changed to detect ResizeObserver

This removes the `WebBrowser. isTooOldToFunctionProperly()` method as it is no longer possible to determine this information based on the user agent.

Fixes #14366
mshabarov added a commit that referenced this pull request Nov 8, 2023
) (CP: 23.3) (#16024)

Changed to detect ResizeObserver

This removes the `WebBrowser. isTooOldToFunctionProperly()` method as it is no longer possible to determine this information based on the user agent.

Fixes #14366

Co-authored-by: czp13 <61667986+czp13@users.noreply.github.com>
Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
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.

Use feature detection instead of browser version detection for the "too old" page
5 participants