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

Diff display suppression should not trigger on long lines #1826

Closed
1 task done
silverwind opened this issue May 28, 2017 · 5 comments · Fixed by #1845
Closed
1 task done

Diff display suppression should not trigger on long lines #1826

silverwind opened this issue May 28, 2017 · 5 comments · Fixed by #1845
Labels
type/enhancement An improvement of existing functionality
Milestone

Comments

@silverwind
Copy link
Member

silverwind commented May 28, 2017

Description

Diff with long lines is errounously detected a being "too large", e.g. should only trigger on char count:

Actual diff ist just 2 (long) lines:

diff --git a/nginx.conf b/nginx.conf
index 568178d..8890cd3 100644
--- a/nginx.conf
+++ b/nginx.conf
@@ -1,2 +1,2 @@
-ssl_ciphers 'ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256';
-# ssl_ciphers "ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS";
+# ssl_ciphers 'ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256';
+ssl_ciphers "ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS";
@andreynering
Copy link
Contributor

Diff with long lines is errounously detected a being "too large".

It's not errounosly, it's by design. The issue is that lives with many chars make the browser slow or even hang.

An example of this is a minified JS or CSS. It's often a single line, and your browser will hang.

You can change that in your app.ini. We can maybe discuss change the default value.

@silverwind
Copy link
Member Author

Right, my example is 681 characters, while the default limit is 500 chars on a single line. I'd say a value like 2000 or more would be a more reasonable default (Quite hard to find a minfied script that is not at least that length).

@andreynering
Copy link
Contributor

andreynering commented May 29, 2017 via email

@lunny lunny added the type/enhancement An improvement of existing functionality label May 29, 2017
@silverwind
Copy link
Member Author

silverwind commented May 29, 2017

Line length alone isn't what makes browsers slow. Slowness is from my oberservation based purely on total character amount, or more specifically, the amount of DOM nodes rendered.

I'm leaning towards increasing it even more, maybe 5k. That's what I currently use in my Gitea instance.

@DblK
Copy link
Member

DblK commented May 29, 2017

I think it's linked to #1827

silverwind added a commit to silverwind/gitea that referenced this issue May 31, 2017
Tests indicate that line length alone does not make browsers slow, so
increase the default threshold after which diffs get surpressed for line
length from 500 to a more reasonable 5000 characters.

Fixes: go-gitea#1826
appleboy pushed a commit that referenced this issue Jun 1, 2017
Tests indicate that line length alone does not make browsers slow, so
increase the default threshold after which diffs get surpressed for line
length from 500 to a more reasonable 5000 characters.

Fixes: #1826
@lunny lunny added this to the 1.2.0 milestone Jun 1, 2017
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants