[PATCH 7 of 7 V5] hgweb: expose a followlines UI in filerevision view (RFC)

[Denis Laxalde]

Gregory Szorc a écrit :
> On Sat, Mar 25, 2017 at 12:34 PM, Gregory Szorc <gregory.szorc at gmail.com>
> wrote:
>
>> On Sat, Mar 25, 2017 at 2:21 AM, Denis Laxalde <denis at laxalde.org> wrote:
>>
>>> # HG changeset patch
>>> # User Denis Laxalde <denis.laxalde at logilab.fr>
>>> # Date 1489594320 -3600
>>> #      Wed Mar 15 17:12:00 2017 +0100
>>> # Node ID ec77aa4ff2993057b604bdffb449d2d179525a9f
>>> # Parent  2f75006a7f31c97d29fd6dd9b72fa7cc9e7ab840
>>> # Available At https://hg.logilab.org/users/dlaxalde/hg
>>> #              hg pull https://hg.logilab.org/users/dlaxalde/hg -r
>>> ec77aa4ff299
>>> # EXP-Topic linerange-log/hgweb-filelog
>>> hgweb: expose a followlines UI in filerevision view (RFC)
>>>
>>> In filerevision view (/file/<rev>/<fname>) we add some event listeners on
>>> mouse selection of <span> elements in the <pre class="sourcelines"> block.
>>> Those listeners will capture the range of mouse-selected lines and, upon
>>> mouse
>>> release, a box inviting to follow the history of selected lines will show
>>> up.
>>> This action is advertised by a :after pseudo-element on file lines that
>>> shows
>>> up on hover and invite to "select a block of lines to follow its history".
>>>
>>> All JavaScript code lives in a new "linerangelog.js" file, sourced in
>>> filerevision template (only in "paper" style for now).
>>>
>>> This is proposal implementation, comments welcome on any aspects.
>>>
>>
>> This feature is frigging awesome!!! I can pretty much guarantee that some
>> engineers at Mozilla will go completely crazy for this feature.
>>
>> As I'm playing with it locally, I found a few paper cuts.
>>
>> First, on the initial revision introducing lines (last revision as
>> rendered in hgweb), the diff is often (always?) empty. I can also see empty
>> diffs in the intermediate changesets. For example, run this on
>> mercurial/exchange.py and ask it for the line range of the
>> _pushdiscovery(pushop) function. For this function, I get 2 empty diffs
>> (revisions 233623d58c9a and ddb56e7e1b92). Highlighting
>> _pushdiscoverychangeset() yields only an empty initial diff.
>>
>> Second, the highlighting UI is a bit confusing. It took me a few seconds
>> to realize I had to actually hold down the mouse button and highlight the
>> lines. I don't think users are accustomed to do this on text in web pages
>> unless they are copying text. It was definitely surprising to me that
>> highlighting lines of text resulted in a special pop-up appearing. I'm no
>> web design expert, but how about this design.
>>
>> As you mouseover lines, you see a graphic cursor somewhere on that line.
>> There is likely a label next to it saying "click anywhere to follow from
>> this line" or something like that. This is similar to the floating text
>> label you have now. When you mouse click, that floating cursor is dropped
>> and locked into place on that line. On subsequent mouseovers, another
>> cursor+label appears. The lines between the initial cursor and the current
>> mouse location are highlighted somehow (possibly adding a different
>> background color). The label says "click anywhere to follow lines XX to YY"
>> or something. When the user clicks to drop the 2nd cursor, either they're
>> taken directly to the filelog view or we get an inline box with links (like
>> the line number mouseovers on the annotate page). If the user accidentally
>> clicks to drop a cursor, they can clear the cursor be clicking the cursor
>> or an "x" next to it.
>>
>> Another minor nitpick with the highlighting UI is the "follow lines" box
>> may not appear next to the mouse cursor. I think we want it to appear near
>> the cursor so it is easier to find/use.
>>
>> The UX can obviously be improved as a follow-up. I don't want to detract
>> from getting the core of the feature landed. This is shaping up to be
>> pretty amazing!
>>
>
> There's also a performance issue on very large files (such as
> layout/base/nsCSSFrameConstructor.cpp from a Firefox repo) in Firefox. When
> you start highlighting, the browser slows down dramatically. This doesn't
> happen in Chrome. (But I can't get the pop-up box to render in Chrome.)
> This is definitely a regression from this patch.

(chrome issue is because Element.setCapture is not implemented there 
apparently, but MDN does not mention this.)

> FWIW, introducing poorly-performing code in a web-based tool that Firefox
> developers use is a good way to make Firefox faster. I've seen web
> performance issues in hgweb and ReviewBoard result in performance
> improvements to Firefox. Funny how that works.
>

This appears to be because of the following CSS rule, more specifically
the :hover selector on a large number of pseudo-elements.

>>> diff --git a/mercurial/templates/static/style-paper.css
>>> b/mercurial/templates/static/style-paper.css
>>> --- a/mercurial/templates/static/style-paper.css
>>> +++ b/mercurial/templates/static/style-paper.css
>>> @@ -276,10 +276,28 @@ td.annotate:hover div.annotate-info { di
>>>    float: left;
>>>  }
>>>
>>> +div.overflow pre.sourcelines > span:hover:after {
>>> +  content: "select a block of lines to follow its history";
>>> +  display: inline;
>>> +  float: right;
>>> +  line-height: 1em;
>>> +  background-color: #FFFFFF;
>>> +  color: #001199;
>>> +}
>>> +