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

Different warning when Weebassembly is not enable #660

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Different warning when Weebassembly is not enable #660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36e318a
specific webassembly warning
PhieF Jun 7, 2024
de3eb4b
missing css style
PhieF Jun 7, 2024
32532af
Merge branch 'marius-wieschollek:master' into jit-warning
PhieF Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions src/js/Helper/compatibility.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,32 @@ function showBrowserCompatibilityWarning() {
throw new Error('Browser does not suport ECMAScript 2017 / ES2017');
}

function isWebAssemblyEnable(){
if(!window.hasOwnProperty('WebAssembly') || typeof window.WebAssembly.instantiate !== "function") {
console.error('WebAssembly not supported');
return false;
}
}

function showWebAssemblyCompatibilityWarning() {

var imgpath = OC.filePath('passwords', 'img', 'warning.png');
container = document.getElementById('main');
Copy link
Owner

Choose a reason for hiding this comment

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

This should be either var container or the previous line should have a comma. I don't think you need imgpath at all.

container.innerHTML =
'<div class="passwords-browser-compatibility passwords-jit-compatibility">' +
'<div class="message"><img class="warning-icon" src="' + imgpath +'"/>'+OC.L10N.translate('passwords', 'To view this website properly, please enable Javascript JIT.') +
Copy link
Owner

Choose a reason for hiding this comment

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

This is a broken image here. But i also don't think we need a warning icon for this.

'</div><div class="info" >' +
'<h3 class="howto">' + OC.L10N.translate('passwords', 'How to enable it') + '</h3>' +
'<p>' + OC.L10N.translate('passwords', 'JavaScript JIT might be disabled in your browser in order to render web content in a more secure configuration. You can always enable Javascript JIT in the settings, depending on your web browser.') + '</p><br />' +
Copy link
Owner

Choose a reason for hiding this comment

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

This text is placed directly onto the background image. This could be unreadable.
Its also very verbose. Additionally, neither Chromium nor Firefox do have a setting for this, both put it into the config / flags page. Additionally, calling the feature JIT may be cause them to misconfigure things in FF as typing JIT into the search of about:config will not show the actual setting to disable WASM.
I feel like it would be better to keep the message short and just tell users that "This app requires WASM/WebAssembly to be enabled. Please check our Wiki to learn how to enable it" and then create a page in the wiki like /Enable-WebAssembly. This would also have the benefit that admins can provide a custom help page instead of users doing things they find on Google

'</div></div>';
container.setAttribute('class', '');

throw new Error('Browser does not suport WebAssembly');
}

function checkSystem() {
if(!isCompatibleBrowser()) showBrowserCompatibilityWarning();
if(!isWebAssemblyEnable()) showWebAssemblyCompatibilityWarning();
else if(!isCompatibleBrowser()) showBrowserCompatibilityWarning();
}

window.addEventListener('DOMContentLoaded', checkSystem, false);
window.addEventListener('DOMContentLoaded', checkSystem, false);
27 changes: 27 additions & 0 deletions src/scss/Partials/_compatibility.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#main {
.passwords-https-report,
.passwords-jit-compatibility,
.passwords-browser-compatibility {
position : absolute;
top : 55px;
Expand Down Expand Up @@ -118,5 +119,31 @@
list-style : disc;
}
}

.info {
max-width: 400px;
margin: auto;
color: var(--color-main-text);
padding: 0px 24px 24px 24px;
}

.warning-icon{
margin-right: 4px;
height: 24px;
}

.howto{
color: var(--color-main-text);
font-weight: bold;
font-size: 20px;
}

}

.passwords-jit-compatibility{
.message{
display: flex;
justify-content: center;
}
}
}