From mboxrd@z Thu Jan  1 00:00:00 1970
From: john at keeping.me.uk (John Keeping)
Date: Sun, 24 Jun 2018 14:39:35 +0100
Subject: [PATCH v3 6/6] line-range-highlight: copy URL to clipboard on
 click
In-Reply-To: <26302F2E-37BA-4457-8713-23B6372B8DBE@warmcat.com>
References: <152980795804.2873.3939128717470466784.stgit@mail.warmcat.com>
 <152980829459.2873.7085053956237952034.stgit@mail.warmcat.com>
 <20180624114233.GL6584@john.keeping.me.uk>
 <26302F2E-37BA-4457-8713-23B6372B8DBE@warmcat.com>
Message-ID: <20180624133935.GN6584@john.keeping.me.uk>

On Sun, Jun 24, 2018 at 08:00:08PM +0800, Andy Green wrote:
> 
> 
> On June 24, 2018 7:42:33 PM GMT+08:00, John Keeping <john at keeping.me.uk> wrote:
> >On Sun, Jun 24, 2018 at 10:44:54AM +0800, Andy Green wrote:
> >> Since the only reason to click on the line number links
> >> is to get the corresponding #URL to share, this patch
> >> makes that process more convenient by copying the
> >> highlit area, be it a single line or a range, to the
> >> clipboard on each click of the line number links.
> >
> >As a user, I'd find this surprising and probably quite annoying.
> >
> >I strongly prefer that software not overwrite the clipboard contents
> >without an explicit request to do so.  A quick survey suggests none of
> >GitHub, Gitlab or repo.or.cz (GitWeb) behave in this way.
> 
> Is there another possible intention behind clicking on the line number
> links I am missing?

One I can think of (given your recent patches) is to highlight a line
when pointing it out to someone else looking at the same display.  I
have definitely used this with other web interfaces in the past.

> If not, literally the only reason to click on them is because you
> intend to copy the #URL to the clipboard.
> 
> Then that is "an explicit request", isn't it?

In the X11 world there are separate selection buffers for selecting text
and for Ctrl-C/V behaviour [1].  It might not be unreasonable to set
PRIMARY to the URL, but setting CLIPBOARD is clearly wrong.

My use of "explicit" above is consistent with the use in this spec,
unless the user has selected Copy, or pressed Ctrl-C there is no
expectation that the clipboard will be overwritten.

Certainly for an X11 user overwriting CLIPBOARD when clicking a
hyperlink fails the principle of least surprise, and I think that
applies to all users because this isn't how web pages work: on any other
page, if I want to copy a link I right-click and copy the link.  I don't
expect the clipboard to be overwritten just because I clicked a
hyperlink.

[1] https://specifications.freedesktop.org/clipboards-spec/clipboards-0.1.txt

> >> Signed-off-by: Andy Green <andy at warmcat.com>
> >> ---
> >>  cgit.js |   36 ++++++++++++++++++++++++++++++++----
> >>  1 file changed, 32 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/cgit.js b/cgit.js
> >> index 2cfad29..e2c3799 100644
> >> --- a/cgit.js
> >> +++ b/cgit.js
> >> @@ -89,8 +89,29 @@ function cgit_line_range_highlight()
> >>  		e.scrollIntoView(true);
> >>  }
> >>  
> >> +function cgit_copy_clipboard(value)
> >> +{
> >> +	var inp = document.createElement("textarea");
> >> +	var e = document.getElementById("linenumber_td");
> >> +
> >> +	inp.type = "text";
> >> +	inp.value = value;
> >> +	/* hidden style stops it working for clipboard */
> >> +	inp.setAttribute('readonly', '');
> >> +	inp.style.position = "absolute";
> >> +	inp.style.left = "-1000px";
> >> +
> >> +	e.appendChild(inp);
> >> +
> >> +	inp.select();
> >> +
> >> +	document.execCommand("copy");
> >> +
> >> +	inp.remove();
> >> +}
> >> +
> >>  function cgit_line_range_click(e) {
> >> -	var t = e.target.id;
> >> +	var t = e.target.id, cp;
> >>  
> >>  	cgit_line_range_override = null;
> >>  
> >> @@ -101,13 +122,13 @@ function cgit_line_range_click(e) {
> >>  	 * is called, and override it there.
> >>  	 */
> >>  
> >> -	if (window.location.hash && window.location.hash.indexOf("-") < 0)
> >> +	if (window.location.hash && window.location.hash.indexOf("-") < 0)
> >{
> >>  		if (parseInt(window.location.hash.substring(2)) <
> >>  		    parseInt(t.substring(1))) /* forwards */
> >> -			cgit_line_range_override =
> >> +			cp = cgit_line_range_override =
> >>  				window.location + '-' + t.substring(1);
> >>  		else /* backwards */
> >> -			cgit_line_range_override =
> >> +			cp = cgit_line_range_override =
> >>  				window.location.href.substring(0,
> >>  					window.location.href.length -
> >>  					window.location.hash.length) +
> >> @@ -115,6 +136,13 @@ function cgit_line_range_click(e) {
> >>  					window.location.href.substring(
> >>  						window.location.href.length -
> >>  						window.location.hash.length + 2);
> >> +	} else
> >> +		cp = window.location.href.substring(0,
> >> +                                        window.location.href.length
> >-
> >> +                                        window.location.hash.length)
> >+
> >> +			'#n' + t.substring(1);
> >> +
> >> +	cgit_copy_clipboard(cp);
> >>  }
> >>  
> >>  /* line range highlight */
> >> 
> >> _______________________________________________
> >> CGit mailing list
> >> CGit at lists.zx2c4.com
> >> https://lists.zx2c4.com/mailman/listinfo/cgit
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit