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
- Create a shared style module that is included in every custom element
- Add the shared style module to each existing element
Change-Id: I1ee382955afe4ff630548a6640e7c4d03688849d
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.mdhttps://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
In the previous fix, the function was not called and since it was
the function that set the class to 'hidden' it did not hide.
Change-Id: Ic8f908802cb515086860f20e2c82d0ddd06c0278
There was a formatting issue caused by delete buttons in a draft change.
It really does not make sense for the button to be there for drafts
anyway. This change hides the delete button for admins on draft changes.
Bug: Issue 6257
Change-Id: I236eff9db1f0ca76ed658f3048132c9c675ee407
Previously, there was a race condition in which erasing a draft comment
could occur prior to the updated version of the comment getting saved.
When this happened, if the user replied to the same thread, their old
draft would be pre-populated in the new reply.
This change cancels the 'store' debouncer when erasing a draft comment,
to avoid this race condition.
Bug: Issue 6141
Change-Id: I11a646acf01b1ac986c587e1e8fdb4fd7efc25b7
Previously, the rest api interface set '__isOnParent' for comments.
When comments were added, the property from the comment thread
'isOnParent' was passed as a property. When the draft was saved, it was
expecting '__isOnParent' rather than 'isOnParent' and this caused
comments to show up on the wrong side after saved/refreshed, because
'__isOnParent' was undefined.
Instead of changing to either '__isOnParent' which would be strange to
pass as an attribute or 'isOnParent' which would look like it came from
the api directly, this change introduces isOnParent functions so that
translation doesn't need to be done with the API.
Bug: Issue 5831
Change-Id: I3b849ba5878275cda0a39638626a12bd51341a29
Use '__isOnParent' as a boolean in place if 'side' ('PARENT vs
'REVISION'). In doing so, it's necessary to convert to/from 'side'
whenever interacting with the REST API.
Change-Id: Ic023c9be1969597e4b9c73a51cfed9f5eb9bc23e
This change introduces an element that can wrap a tooltip around
arbitrary contents. Previously, we could only attach a tooltip to a
Polymer element, and want more granularity than that for the purposes
of positioning.
The new tooltip element takes the following arguments:
- title (the contents of the tooltip, required)
- showIcon (optional, to show an info icon)
- maxWidth (optional, to pass a max-width to the tooltip itself).
The tooltip itself will be attached to the content inside of the
<gr-tooltip-content> wrapper.
One tooltip has been added using this new element for the 'DRAFT' text
in the gr-diff-comment'.
Bug: Issue 4539
Change-Id: Iff05c785052c643ef1f1cc01d101b21c48c6299f
Previously, if a PARENT revision was getting compared to a patchset,
the local storage key would be the same for both, and there were issues
if a user was trying to write drafts on the same line on either side.
This addresses the issue by storing 'PARENT' as the patch number in the
local storage key, so that each key is unique.
Bug: Issue 5412
Change-Id: Ia8b2f0abe1d24a3849628fe335c931f07bcaff52
Removes the timezone from the timestamps, as the tooltip shows UTC
offset on mouseover.
Also upgrades most instances of gr-date-formatter to use gr-tooltip.
Bug: Issue 5587
Change-Id: I57c7dcacf0618ffd967eff3cb4ff37a5e1876180
There was an issue where gr-formatted-text would not display when the
config was not loaded yet. This change adds a function to display text
as is, in the event that the config is not yet loaded. It also refactors
the existing functions to make it more clear where config is needed.
Bug: Issue 5690
Change-Id: I74896bd59793b26b2b8fe289c13e5762c60fe8df
Move all buttons that generate a reply of some sort (done, ack, reply,
quote) to the comment thread instead of the comment [1].
When there is a draft for a particular comment thread, all reply buttons
are hidden [2]. For example, if you click reply, you cannot click done
on the same thread, unless you remove the draft.
Each thread can have up to 1 draft. It's also worth noting that if a
thread has a draft, and the user clicks on the line or 'c' at the same
range, the existing draft will switch to 'editing' form.
[1] With the exception of "please fix" for robot comments.
[2] In this case, The please fix button will be disabled when other
reply buttons are hidden.
Feature: Issue 5410
Change-Id: Id847ee0cba0d0ce4e5b6476f58141866d41ffdad
Goes along with c/95273/. Adds commentSide attribute to comments to see
which side of the diff view they belong on. This is also used as part
of the locationRange for the gr-diff-comment-thread-group, so that two
thread groups can be on the same line or range for the unified group (
one for the right, one for the left).
Note: there is already a 'side' attribute on the gr-diff-comment, which
is confusing. This side actually references 'PARENT' or 'REVISION', to
identify whether the comment belongs to the parent or any revision. On
diffs where two revisions are compared to each other, this cannot be
used to determine left/right. However, because 'side' is part of
the CommentInfo entity[1], it is difficult to change the name and make
more sense out of that.
[1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#comment-info
Bug: Issue 5114
Change-Id: I5cc4c17d4bb134e31e5cc07ff9b08ed349488c97
Previously, drafts were only based on line number and not patch range.
This adds range to the key in localstorage so that multiple drafts can
be stored for a line range. It also prevents the wrong draft from
surfacing in the incorrect context.
This change is also important for the multi-thread changes that are
currently in development as well, because it is possible to have
separate threads for these ranges, each of which can have its own draft
saved in storage.
Bug: Issue 3548
Change-Id: Id4c1beb5d73a47a1f98b0b169091905c80f8c64a
Adds the 'resolved' checkbox to the front end. Unresolved comment
threads are indicated by #fcfaa6 as background-color, whereas resolved
threads have the background #fcfad6.
Feature: Issue 4879
Change-Id: Ie1eeba61ccba559f89b707542acab2198c99b8a7
This change leverages the unresolved flag to automatically expand only
the last UNRESOLVED_EXPAND_COUNT comments in unresolved threads.
Feature: Issue 4752
Change-Id: Ia23920e1a210246838645d56a6bc81d0dff7da07
This change tracks and exposes the resolved state of a comment thread
without exposing the UI for modifying that state. This enables features
to be built out while the API request does not exist in the backend.
Feature: Issue 4879
Change-Id: If002035024920a7762519cedf5a869221bbbc3c8
This change addresses a few issues that existed due to multiple comments
in an editing state at the same time.
1) Fixes issue where if you create two replies and add text to the
second reply, then delete the first one, the text in the second textarea
gets removed.
2) Fixes issue where if you reply and add text, then reply again with
the first draft still editing, the second draft gets populated with the
message from the first comment.
3) Fixes issue where if you have multiple replies and delete one of
them, local storage gets erased. This change sets local storage to the
value of the first editing message found after deleting the other one
(if it exists).
Bug: Issue 4409
Change-Id: Ib5913a34a79783a4a87b4a298e25b02fc587b8dd
This change adds an API request to get robot comments for displaying
inline in the diff view. They are styled in a different color, contain
build and robotId information, and a "please fix" action rather than the
standard set of actions.
Feature: Issue 5089
Change-Id: I1f5954a2ed01920bb7c3dc897e3285687ff7d3ca
Previously, when a user discarded a draft comment, the draft was still
stored in local storage. If they tried to reply to a comment on the same
line, the previous message appeared when it shouldn't have. This change
removes drafts from local storage when they are discarded.
Bug: Issue 4361
Change-Id: Id234676972a21a879e68e754968abf7008098cf6
+ This does not cover on-keydown handlers within elements.
A follow-up change will account for those.
+ Keyboard shortcuts are disabled within gr-overlay, input,
and textarea elements.
+ Added tests for new behavior (plus some missing ones covering
broken behavior).
+ Removed blur hacks used on elements to placate the kb
shortcuts due to restrictions that have been removed.
Bug: Issue 4198
Change-Id: Ide8009a3bfc340a35a8ec8b9189a85b49c8a95aa
This change is the PolyGerrit counterpart to [1].
Nicer rendering for reviewer comments in PolyGerrit using the Gerrit
Wiki-like format. Whereas, formerly, PG comments were set in PRE blocks
using monospaced font so that the original format and alignment of the
comment can be directly viewed. This change allows comments to default
to a variable-width font with wrapping while separately styling blocks
intended to be pre-formatted text, quotes and lists.
The logic to parse comment text into blocks is borrowed from the Java
implementation found in [1]. Test cases are additionally translated from
this change to ensure coincident behavior.
Introduces GR-FORMATTED-TEXT to display these comments, and which uses a
similar interface to GR-LINKED-TEXT. Much like [1], the comment is
parsed into a list of blocks. These blocks are then mapped to the DOM
nodes that get attached inside the element.
[1] I8e11d363b80bff0b6395f56e210b636f68db36fa
Feature: Issue 4861
Change-Id: I245d6782e2fd8982ac3eda438fe4ca80f3658195
Previously, iron-autogrow-textareas did not prompt autocomplete on
mobile devices. With this change, mobile browsers that support
autocomplete will suggest words to the user.
Bug: Issue 4683
Change-Id: I9286b998df95798f88c56f66fd63d731e5a88cc5
Previously, the escape key closed and cancelled any in progress comment.
This was particularly painful for some who were used to a certain text
editor, so now escape only closes an in-progress comment if it's empty.
Bug: Issue 4816
Change-Id: I2749a995ebf914c4387261c387b008f6ce91dd86
This change adds keyboard shortcuts to the "gr-diff-comment-thread"
expand all comments when 'e' is pressed and collapse all comments when
'shift + e' is pressed. Note that the keyboard event is detected on the
thread instead of the comment to minimize the number of events getting
triggered.
Feature: Issue 4738
Change-Id: Iab77349bd1527d7af5e05a827919a78a86909835
Previously in Polygerrit, comments were always expanded. You could
always see the full comment (if multiline) and any applicable actions.
This change creates a collapsed comment view. It adds a preview of the
text to the header row when collapsed, and can be toggled open when any
part of the header is clicked.
Bug: Issue 4698
Change-Id: Idca5caf92eb32518b6737dbb5a3380d227513996
Shortcut only functions while editing the actual text area of the
comment. This makes sense, as one should have to save each individual
comment as a draft. This also mimics the current behavior of GWT UI.
Bug: Issue 3848
Change-Id: I7037d3dbdc5dae3048fcae580ed09c41e54f0669
Apply diff annotations (intraline differences and comment ranges) by
executing the annotation layers in order to each line. The diff builder
maintains an ordered array of annotation layers which are communicated
to GrDiffBuilder subclass instances. The builder also listens to each
layer for notifications that annotations have changed for some line
range and re-renders (i.e. re-applies the pipeline on DIV.contentText
elements) accordingly.
Change-Id: Iea0599d4869cafaadc0974158153a91d927913e8
