New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 624368 link

Starred by 4 users

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 631058



Sign in to add a comment

80-character-wide CL descriptions should not cause scroll bar to appear

Project Member Reported by serg...@chromium.org, Jun 29 2016

Issue description

See https://chromium-review.googlesource.com/c/357040/. The longest line in description is 80 characters long. There is plenty of space on my screen and I don't see a reason to put a scrollbar in the first place, but even if it's there, it should be wide enough to include 80 characters.

Screenshot: https://screenshot.googleplex.com/Lrro1ESeb8B.
 
Status: Available (was: Untriaged)
Components: Infra>Codereview>Gerrit
Labels: Pri-2
Status: WontFix (was: Available)
Gerrit commit message style is maximum 72 chars.
Owner: andyb...@chromium.org
Status: Assigned (was: WontFix)
assigning back to Andy to explain why we choose to won't fix this.
+ The 72 char limit that Gerrit imposes for stylistic reasons on commit messages.
+ There needs to be room for the column that shows dependent changes/submitted together/etc.
+ Wrapping the text isn’t an option because, like code wrapping, it prevents someone from seeing that their message does not meet the 72 char limit.
Labels: -Pri-2 Pri-3
Blockedon: 631058
Another screenshot with relation chain: https://screenshot.googleplex.com/SsOmTtdPkHa.png.

I still see enough space to the right of the relation change to extend the desctiption field to be 80 characters wide. Alternatively, we can try and reduce size of the font slightly to accommodate another 8 characters. Also, as seen in https://screenshot.googleplex.com/AJ3JKW9b3wU.png, on Mac one can not even see the horizontal scrollbar by default and it's obvious at the first glance that there is more to the message on the right: https://screenshot.googleplex.com/Xxmf8ydYBSp.png.

The reason I think we should change it because, 80-character width is de-facto stardard, e.g. on my machine git-cl-upload would start vim to enter commit message, which would automatically wrap message by 80 characters.
s/and it's obvious/and it's NOT obvious/
Screenshot with 80ch width: https://screenshot.googleplex.com/H7zYhWn7dff.png. IMHO, it still looks pretty good even with relation chain.
Note that even if we keep description 72ch width, we still can't fit 72-character long description of the CLs in the relation chain: https://screenshot.googleplex.com/nqm3ZPUpYfZ.png.
Labels: Type-Bug
Status: WontFix (was: Assigned)
A change was landed to wrap commit messages for now so that the scroll bar doesn’t appear. You can refile a bug within the Gerrit project for further discussion on the maximum line length.
Project Member

Comment 15 by bugdroid1@chromium.org, Mar 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/783059297a429d72f45dc835a738233139d822f7

commit 783059297a429d72f45dc835a738233139d822f7
Author: dtapuska <dtapuska@chromium.org>
Date: Wed Mar 01 21:25:16 2017

Reset the touch-action on the touch gesture end sequence.

This makes the property that the touch-action is always adjusted on
receiving touch ack. Change https://codereview.chromium.org/2715623002
broke the property that a single touch action was outstanding. This caused
issues with a delayed touch-end handler possibly and another touchstart
being issued the touch-action would have been reset intermitently there.

There is already a test that covers this and it will be apparent when
the test is modified to support rAF aligned input enabled (which then
turns on the pass through touch event queue).

BUG= 624368 

Review-Url: https://codereview.chromium.org/2724743004
Cr-Commit-Position: refs/heads/master@{#454036}

[modify] https://crrev.com/783059297a429d72f45dc835a738233139d822f7/content/browser/renderer_host/input/input_router_impl.cc
[modify] https://crrev.com/783059297a429d72f45dc835a738233139d822f7/content/browser/renderer_host/input/input_router_impl_unittest.cc

Sign in to add a comment