27 Commits

Author SHA1 Message Date
Wyatt Allen
117e449efb Move descendedFromClass into a behavior
Change-Id: Ie592a1b2d22cfc0ba32514fa4a48f68f7ed9eeb6
2018-01-31 18:25:42 -08:00
Wyatt Allen
ee40578526 Handle diff selections that end at the last line
When a diff selection extends to the end of the diff, the range that is
created uses the `contentWrapper` for the `endContainer`. When calling
the `getLineElByChild` method on the `contentWrapper` the result is
`null`. Formerly, checking `null` for the line number attribute would
throw an exception and fall back to native copy.

With this change, the `_getRangeFromDiff` method is taught to accept
`undefined` for the `endLineNum` parameter. The `_getSelectedText`
method does a `null` check and passes `undefined` in this case.

Bug: Issue 7895
Change-Id: I27f42e0e40b6268a35498e72a953415ec97ca9d7
2017-12-08 11:41:15 -08:00
Paladox none
d6a6a8b476 PolyGerrit: Replace content with slot part 1
As slot is replaced with content at runtime, we don't need to adjust any
css. This makes swapping with slot easier. But css will need to updated to be
compatible with 2.x+ at some point later.

Change-Id: Ic0ea6a746a4c959a788a6b0fa0695a58e4b480ee
2017-11-10 23:37:33 +00:00
Wyatt Allen
ed628d7f37 Show blame in diff
With this change a blame column is added to the left side of diff
tables. The column is empty and hidden until blame is loaded. A button
is added to the change view to trigger a load of the blame for that
diff, as well as a unload it if already loaded. In this stage, the blame
information is non-interactive and only displays the SHA, date and
commit author.

Feature: Issue 6075
Change-Id: Ifcb951265d0e6339094e6b7c9574ec9c69e60b51
2017-09-27 18:55:42 -04:00
Becky Siegel
8d92d53db5 Annotation updates
Change-Id: I146f76b9dcc1a92e18acec01481ad280fb431868
2017-08-12 11:49:52 -07:00
Mike Samuel
e07c4b2ea1 Add polygerrit-ui/app/test/common-test-setup.html
This is a partial roll-forward of c/106190

This replaces all loads of iron-test-helpers with a load of a file
that wraps it, and adds that file to test files that do not currently
load iron-test-helpers.

A future CL will also install polymer-resin via common-test-helpers.html.

I tested by running

$ WCT_ARGS="-l chrome" ./polygerrit-ui/app/run_test.sh

Change-Id: Ifb3cd2c8db13d724f57e56e7e78045470d103a43
2017-06-05 22:10:12 +00:00
Becky Siegel
b159a7f5cc Update styles for shadow dom
- Create a shared style module that is included in every custom element
- Add the shared style module to each existing element

Change-Id: I1ee382955afe4ff630548a6640e7c4d03688849d
2017-06-02 14:54:03 -07:00
Wyatt Allen
c601abccc3 Revert "Polygerrit now loads polymer-resin"
This reverts commit 0895052c01ac5ac657a9763d2ad9967d9ae55c18.

Reason for revert: issue 6387

Change-Id: I14e00addeab53606952aa3ea2d45a74eac7a9d8a
2017-06-02 09:37:37 -07:00
Mike Samuel
0895052c01 Polygerrit now loads polymer-resin
polymer-resin intercepts polymer property assignments
before they reach XSS-vulnerable sinks like `href="..."`
and text nodes in `<script>` elements.

This follows the instructions in WORKSPACE for adding a new bower
dependency with kaspern's tweak to use the dependency in a rule so
that it's found.  //lib/js/bower_components.bzl has already been
rolled-back per those instructions.

The license is the polymer license as can be seen at
https://github.com/Polymer/polymer-resin/blob/master/LICENSE though
I'm not sure that //tools/js/bower2bazel.py recognizes it as such.

Docs for the added component are available at
https://github.com/Polymer/polymer-resin/blob/master/README.md
https://github.com/Polymer/polymer-resin/blob/master/getting-started.md

With this change, when I introduce an XSS vulnerability as below,
polymer-resin intercepts and stops it.

Patch that introduces a strawman vulnerability.

--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
@@ -55,6 +55,10 @@
         url: '/q/status:abandoned',
         name: 'Abandoned',
       },
+      {
+        url: location.hash.replace(/^#/, '') || 'http://example.com/#fragment_echoed_here',
+        name: 'XSS Me',
+      },
     ],
   }];

---

Address kaspern's and paladox's comments.

---

Undo version bumps for bower dependencies.

---

Change Soy index template to parallel app/index.html.

---

update polymer-resin to version 1.1.1-beta

----

Load polymer-resin into polygerrit-ui/**/*_test.html

After this, I ran the tests with
  -l chrome
  -l firefox

I ran a handful of tests with -p and observed that the
console shows "initResin" is called before test cases start
executing.

These changes were done programmaticly by running the script below
(approximately) thus:
```
gerrit/ $ cd polygerrit-ui/app
app/ $ find . -name \*test.html | xargs perl hack-tests.pl
```

```
use strict;

sub removeResin($) {
  my $s = $_[0];
  $s =~ s@<link rel="import" href="[^"]*/polymer-resin/[^"]*"[^>]*>\n?@@;
  $s =~ s@<script src="[^"]*/polymer-resin/[^"]*"></script>\n?@@;
  $s =~ s@<script>\s*security\.polymer_resin.*?</script>\n?@@s;
  return $s;
}

for my $f (@ARGV) {
  next if $f =~ m@/bower_components/|/node_modules/@;

  system('git', 'checkout', $f);
  print "$f\n";

  my @lines = ();
  open(IN, "<$f") or die "$f: $!";
  my $maxLineOfMatch = 0;
  while (<IN>) {
    push(@lines, $_);
    # Put a marker after core loading directives.
    $maxLineOfMatch = scalar(@lines)
      if m@/webcomponentsjs/|/polymer[.]html\b|/browser[.]js@;
  }
  close(IN) or die "$f: $!";

  die "$f missing loading directives" unless $maxLineOfMatch;

  # Given ./a/b/c/my_test.html, $pathToRoot is "../../.."
  # assuming no non-leading . or .. components in the path from find.
  my $pathToRoot = $f;
  $pathToRoot =~ s@^\.\/@@;
  $pathToRoot =~ s@^(.*?/)?app/@@;
  $pathToRoot =~ s@\/[^\/]*$@@;
  $pathToRoot =~ s@[^/]+@..@g;

  my $nLines = scalar(@lines);
  open(OUT, ">$f") or die "$f: $!";

  # Output the lines up to the last polymer-resin dependency
  # loaded explicitly by this test.
  my $before = join '', @lines[0..($maxLineOfMatch - 1)];
  $before = removeResin($before);
  print OUT "$before";

  # Dump out the lines that load polymer-resin and configure it for
  # polygerrit.
  if (1) {
      print OUT qq'<link rel="import" href="$pathToRoot/bower_components/polymer-resin/standalone/polymer-resin-debug.html"/>
<script>
security.polymer_resin.install({allowedIdentifierPrefixes: [\'\']});
</script>
    ';
  }

  # Emit any remaining lines.
  my $after = join '', @lines[$maxLineOfMatch..$#lines];
  $after = removeResin($after);
  $after =~ s/^\n*//;
  print OUT "$after";

  close(OUT) or die "$f: $!";
}
```

---

update polymer-resin to version 1.2.1-beta

---

update Soy index template to new style polymer-resin initialization

----

fix lint warnings

----

Load test/common-test-setup.html into *_test.html

Instead of inserting instructions to load and initialize polymer-resin into
every test file, add a common-test-setup.html that does that and also fold
iron-test-helpers loading into it.

----

imported files do not need to load webcomponentsjs

Change-Id: I71221c36ed8a0fe7f8720c1064a2fcc9555bb8df
2017-05-30 23:16:09 -04:00
Kasper Nilsson
2f07624fd4 ES6ify /gr-diff-selection/*
Bug: Issue 6179
Change-Id: I2c929b863c22dc28df7b571002d6ea927b8e631e
2017-05-17 11:12:06 -07:00
Kasper Nilsson
7aebc5fdc8 Fix Closure errors in PolyGerrit
The Closure Compiler is very picky with regard to JSDoc formatting. This
change fixes a few invalid JSDoc issues, and removes the corresponding
suppresses from the BUILD file. Additionally, modify the transpilation
target language to ES5.

After this change, there should no longer be warnings from the Closure
Compiler during building of PolyGerrit.

Change-Id: If7566e40c2c419c55f2a634c0f558c6cc263f61c
2017-04-27 09:00:43 +02:00
Viktar Donich
29e1ce5e84 Collection of prospective test flake fixes
Potentially related:
https://github.com/Polymer/web-component-tester/issues/505

Bug: Issue 5792
Change-Id: I9ab6e8e40d9811dd52906335426764c052907609
2017-03-30 13:46:58 -07:00
Kasper Nilsson
56961e7039 Purge copy/paste cache when diff changes
Bug: Issue 5441
Change-Id: Ifa180dbb4acedce17bc7b75186386d539de5b80e
2017-02-17 00:31:07 +00:00
Kasper Nilsson
13ab50938a Fix copy/paste edge case
In some cases, even though the copy selection was a valid one, the
target is something existing outside the content element. One specific
instance of this is when selecting lines by clicking/dragging along line
numbers.

Bug: Issue 5325
Change-Id: I3496a9d2432201aae6ef43a63ccca601a3bcd309
2017-01-24 09:50:18 -08:00
Kasper Nilsson
61e253ca6a Fix copy/paste in comments
Previous comment logic relied on the assumption that any element in
a comment with textContent would be childless, which is false. Now, the
method is to fetch all comments within the selection and recursively
extract their text content.

Bug: Issue 5144
Change-Id: I22eac0b9b147fe0f3360d87529661efadb15940a
2016-12-20 10:49:58 -08:00
Kasper Nilsson
1ba850e594 Fix comment copy logic
The addition of formatting in comments broke a variety of things having
to do with the copying logic. This change updates the logic and tests
to reflect the new DOM.

This issue arose because of a lack of integration tests for copying and
selection. That test is coming in a descendant change.

Bug: Issue 4969
Change-Id: I4e1994ab07947506c77b07877a46a9369d666d50
2016-12-02 15:25:27 -08:00
Kasper Nilsson
bc0c634347 Fix copy within same line
When the text selection exists within one line in diff-view, the
derivation of the selected text from the diff object using substring()
mutated the text and caused some selections to be artificially long.
Swapping the calls so that the end of the string is trimmed off first
fixes this.

This change also includes a regression test for this issue.

Bug: Issue 4794
Change-Id: I19dbab1cddd2e522cc6495011e8ef49f8ffe55b2
2016-10-20 10:30:55 -07:00
Kasper Nilsson
848371e628 Fix copy behavior in comment textarea
Deferring to default copy behavior when copying within diff comment
textareas allows the browser to handle copying. Also includes a
regression test.

Bug: Issue 4624
Change-Id: Ib976dd5055cf0ede89591ab2f483aa8d264a5484
2016-10-13 13:55:05 -07:00
Kasper Nilsson
a742ae3999 Fix DOM traversal of copy process for comments
The comment selection process was being performed incorrectly due to
lack of utilization of Polymer.dom. This uncovered a few more issues
with the way the fake copy events in testing were being constructed and
a base case in comment text selection.

Bug: Issue 4674
Change-Id: I7e05b7ebe0bfd60bc9cc8973166685c24e25ab37
2016-10-04 17:57:23 -07:00
Kasper Nilsson
708d34fb1f Fix comment selection and copy behavior
This change adds logic that applies a CSS class whenever mouseDown
occurs inside of a comment thread that makes comment messages
highlightable. This replicates the existing functionality of GWTUI. In
addition, logic for copying the messages of multiple comments at once
is included.

Bug: Issue 4494
Change-Id: Id8e49e500c8d48dbba661b885050657dd4cfc0be
2016-09-16 13:51:13 -07:00
Wyatt Allen
b1fbd9b310 Normalize selection ranges for copy
At the time that syntax highlighting DOM was introduced, the offsets of
selection ranges had been broken. In change [1] Kasper fixed this for
GR-DIFF-HIGHLIGHT with selection normalization functions. However,
selections for copying code as implemented in GR-DIFF-SELECTION were
still un-normalized.

With this change, the normalization functionality introduced in [1] is
moved to a JS library so that it can be used by both components. Tests
are updated.

[1] I26c61ca706575ea5df6e3b7b18a27225834396e8

Change-Id: I35ab0f71a46b3fc1d7356a314a0cae856f2ef28e
2016-09-14 10:13:46 -07:00
Wyatt Allen
9fcec7437f Generate clipboard data via diff rather than DOM reconstruction
This change uses a new method of generating the clipboard data that
circumvents several strange issues caused by shadow/shady DOM. This
new method is much more legible and abstracts more logic away from the
DOM.

Bug: Issue 4494
Change-Id: I8c186d6cbbe9536548d934f734856b1f9ced1a26
2016-09-13 14:24:57 -07:00
Kasper Nilsson
8bfab3ff24 Hotfix Safari WCT issues
This change fixes two minor issues in Safari.
- In gr-diff-highlight, Safari does not support NodeList.forEach.
- In gr-diff-selection_test, a test that should have been failing (a
  selection class was missing) was not being run in Chrome/CI. A prior
  commit modified the CSS selectors that enable text selection, and the
  test was not changed to reflect this.

  SHA of the offending commit: 613b49c

Change-Id: I7ef3b017be9ae731496609430e3856f58e5bd24e
2016-09-07 23:39:06 +00:00
Kasper Nilsson
613b49c64e Fix copy/paste in diff view
The addition of syntax highlighting silently broke copy/paste
functionality due to the addition of another layer of div nesting.

Related to this bug are some issues with correct text selection in
unified diff view, so this patch addresses them as well.

Bug: Issue 4317
Change-Id: Iac7379de4131ab4e44905a54218d42fcfe67ce62
2016-08-29 16:18:47 -07:00
Wyatt Allen
0490413eb8 Applies optimizations to GR-DIFF-SELECTION
A major source of latency in creating comments in large diffs stems from
GR-DIFF-SELECTION applying selection-restricting classes to a parent of
the diff. When the diff contains a large number of lines (and thus a
large number of elements) applying a class forces a style recompute for
the substantial subtree.

This change optimizes this in two ways:

* **Initialize to the right side:** The selection-restricting class is
  initialized to the right side, before the diff is even completely
  built. This eliminates the need for a recomputation preceding the
  first comment add on the right side (the most-likely side to be
  interacted with).
* **Minimize the number of class modifications:** only add or remove
  classes when necessary. This eliminates recomputes for consecutive
  mousedown events that occur on the a common diff side.

On an i7 MBP, this eliminates ~120ms for the first comment on the right
and ~40ms for subsequent such comments.

Change-Id: Ibb8a7eca0398a5c7265dc1385967bce3dae5e5ef
2016-08-10 11:47:27 -07:00
Viktar Donich
01c211af6e Gr-diff-selection tests to work in Safari
Change-Id: I31e232acf4d53d521d38d6b2b14f5c39146c05d1
2016-06-13 14:32:24 -07:00
Viktar Donich
e1341972d1 Move text selection out of gr-diff.
Change-Id: I0734653066a1bb78f95c141aa8202fad315b13c0
2016-06-10 12:18:25 -07:00