The present symbol is "n-ary times operator" (U+2A09), from
the supplemental mathematical operators block, which renders
a much larger glyph than typical for basic multiplication.
This change replaces it with "multiplication sign" (U+00D7),
from the Latin-1 supplement block. It is the symbol ordinarily
used for multiplication (including dimensions), and is also
likely present in more fonts.
Change-Id: Id0e168bc61e25924e6b651587dc6b19a06c185f7
Highlights:
* Adds a new getDiffLayers function to gr-js-api-interface.js. This is
invoked by gr-diff-builder.html when gathering annotation layers.
* New annotationApi function in gr-public-js-api.js for plugins to call.
* The annotationApi function returns an instance of the new
GrAnnotationActionsInterface in gr-annotation-actions-js-api.js
* GrAnnotationActionsInterface has an API for the plugin to register an
addLayerFunction and an optional method to call to get a notify callback.
* The new samples/coverage-plugin.html is an end-to-end example of how
to invoke the new APIs to annotate lines.
Bug: Issue 7339
Change-Id: Ie51845e0b3564953aba5d7d41986cedce0337073
Binary files cannot be diffed like text, but for some binary changes,
the diff algorithm does yield binary differences as text. With this
change, the diff builder and processor are taught to render a special
message for (non-image) binary diffs, and will ignore the text
representation of the delta.
Bug: Issue 4031
Change-Id: I2dcdbe9def006de827a37c35c42606bc1c9cf4fc
This shows a nearly 4x improvement (in Chrome) for building
the DOM structure for a large diff. (this doesn't include
style calculation and layout, which is still expensive).
Bug: Issue 6514
Change-Id: Id06db0c9a281b846078aef0e6d584e1fa3c03d0f
The <content> element is deprecated, part of the Shadow DOM v0 API
which has been supplanted by Shadow DOM v1's <slot> element. For very
simple uses – like those in this change – this change should be safe
and will not require changes to the usage sites of your element.
Therefore, these changes are backwards-compatible with Polymer 1.0
applications.
As <content> is the more powerful API, Polymer 1.0 supports both
<content> and <slot> by converting <slot> back into <content> at
runtime. Polymer 2.0 only supports <slot>. There will likely never be
cross-browser native support for <content>, while Safari and Chrome
already support <slot> natively.
PiperOrigin-RevId: 174107440
Change-Id: I21fd97ff6efe95231089f98853146353e0e4ebf2
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
Some sub-suites of the gr-diff-builder tests already used Sinon
sandboxes, but no sandbox had been setup for the tests overall. With
this change all tests in this file are upgraded to use the sandbox.
Change-Id: I778f5e68ee5fb8e73e9b5245252b18f934ebfee1
Formerly, if a formatted text component tried to render without the
project config (used by inner linked text components) it would
temporarily fall-back to rendering the unformatted (and un-linkified)
text via `.textContent` -- mirroring the behavior of gr-linked-text.
The result is formatted text elements (when rendered without a project
config) appear as one long line of text. Unlike linkification, however,
text can be accurately formatted with or without the project config --
so this disruptive, poor UX is unnecessary.
The formatted text component is updated to format text when the project
config has not provided, and to re-render when the config has been
provided.
In order to propagate project config loads to the formatted text
components hosted by diff comments, the REST calls must be made by the
diff thread component. To make this call, the thread must have the
project's name, so gr-diff-builder is updated to provide this name to
thread components.
Bug: Issue 6686
Change-Id: I8d09c740930500e99cb5f87b92f4d72f3f50a9ce
The release of eslint 4 introduced some breaking changes -- a full
upgrade guide can be found here [1].
In this change, the "no-useless-escape" and "indent" rules are disabled,
"indent" being replaced with "indent-legacy" to minimize breaks due to
lint. Also, extra spaces before comments and other assorted new lint
issues are corrected.
The long-term plan is to move away from the deprecated "indent-legacy"
rule and also reenable the "no-useless-escape" rule, but for now we
want to minimize breakages and not affect PG developer velocity.
[1] http://eslint.org/docs/user-guide/migrating-to-4.0.0
Change-Id: I8044256c95e3fe3f94fa9acc7922600908f8a0f4
Adds an aria-label to added and removed rows marking the row as "added"
or "removed" for screen readers. As part of this change, the <tr>
elements that make up the diff are now focusable.
Test plan:
- Somewhat included as a part of testing this functionality is the use
of WCT's a11ySuite function[1]. This will be introduced in a later
change.
[1] https://goo.gl/P15eE6
Bug: Issue 6435
Change-Id: I676fc171d404ca6e8e9276965192b1452595fa61
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
Previously, when a diff was reloaded, it would clear the diff content
(the previous diff) and open requests for the data regarding the new
diff. When the requests finished, it would cancel async processing and
start it again on the new data. In this arrangement, a narrow race
condition was present where the async rendering could emit a diff from
the previous diff after the diff table had been cleared, but before the
network requests resolved -- resulting is content form the previous diff
appearing in the new one.
With this change, the async processing is cancelled at the same time
that the diff content is cleared.
Bug: Issue 6170
Change-Id: Ie5a807082ab51156595f9eeb9ba6c958d41e890a
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
Noteworthy decisions:
- Preferences are hidden when diff prefs are not loaded or the user is not
logged in.
- Preferences are hidden on small screens
- The trigger button is in gr-change-view but the gr-diff-preferences
is part of gr-file-list. This is because gr-file-list because diff
preferences and local preferences are more closely tied with that
than the change view. In order to put it in the change view, local
prefs would also have to be two-way bound back.
Also fixes computePrefsButtonHidden in gr-diff-view as well. The
function did not work as intended before. If preferences didn't exist,
the function would not get called, and the container would not be
hidden.
Bug: Issue 5426
Change-Id: I361cdf132c6e15b5ae2f15e62af318cfa05161ce
Runtime errors occurred when image diffs showed added or removed images
(as opposed to modified images). With this change, test cases are added
to encode behavior against future regressions, and diff builder logic is
updated to avoid the error.
Bug: Issue 5995
Change-Id: If04220256074dac245b971e6229f0dae4a175689
Previously there were times when binary data from an image diff would
display. This change adds a check in the diff processor for image diffs
and does not display anything other than the file line in that case.
This change also addresses an issue where the label is calculated too
soon. The image size (if it exists) is supposed to be included as part
of the label, but often this was calculated before the image was done
rendering, so it didn't display.
Bug: Issue 5887
Change-Id: I9cd1ad0c3f2603492d7d84892147bd6852bbae29
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
Previously diff cursors initialized when their diffs completely finished
rendering. This means waiting longer than needed because the diff render
process includes processing and rendering syntax highlights whereas the
diff cursor only needs to wait on the content of the diff.
With this change diffs fire an event when they've finished with their
content only, and diff cursors await this event instead of the full
render.
Bug: Issue 5681
Change-Id: Ida9da83a20fe4c2dcd8920304504ec7e1185eb5d
Previously, the logic was only checking exact line wrap boundaries and
missed tabs advancing over the boundary.
Also, line breaks were added inside span elements wrapping tabs, which
prevented correct rendering.
Bug: Issue 5597
Change-Id: I750547cb574c02965d5a30ba57f791841779297a
Previously, there was an issue where if you create a draft comment in
side by side view and switch to unified view, the comment thinks it's in
the later patch set rather than the earlier one and a second copy gets
added to local storage with the later patchset as a component of the
key.
This was because the the thread group assumed all threads inside of it
had the same patch number. This change fixes that, so in the event that
a user switches from side by side to unified, the patch number will get
taken from the comment rather than the thread group.
Bug: Issue 5493
Change-Id: I7f00997bcb2e6f1001a5d58ac206acf5af3367d2
Changes with large numbers of files could overwhelm PolyGerrit when a
user selects [Expand all] for inline diffs. This was because the
asynchronous processing/rendering/annotating process would be kicked off
for all unexpanded diffs simultaneously, resulting in browser lock-up
and general slowness even after rendering had completed.
With this change, inline diffs are rendered in serial rather than
parallel. In this way the benefits of the async features of diff
rendering extend to the file list, even for changes with many large
diffs (such as the one in the linked issue).
With this change, the `__expanded` property is removed from file objects
in GR-FILE-LIST. Instead, that element maintains a list
(`_expandedFilePaths`) which records the same information. Because the
expanded files are recorded in a list, however, splices on the list can
be observed, batch diff expansion can be handled sequentially.
Tests are updated to respect the new expanded paths list.
Bug: Issue 5396
Change-Id: Ib83ff5157177e1c890db8a82fbc25df8fecbe065
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
- Add concept of diff comment thread groups, which are all of the
threads at a particular line number.
- The thread group is responsible for breaking up comments into threads
based on the range of the comment.
- Thread groups are ordered by the updated time of the first comment in
the group.
- Thread groups are given a key, based on comment range, which is used
to determine what thread group a new comment should go in (or if it
needs a new one).
Feature: Issue 5292
Change-Id: If544e8bb879262de3ce5397e86124837b66ada04
Because the `_computeAccountTitle` method was only used in a binding to
an HTML attribute, the manual HTML escaping performed in that method was
in not needed.
Since removing this, the only use-site for the `util.escapeHTML`
function is the `GrDiffBuilder`. To discourage general use of manual
HTML escaping (in favor of default escaping in Polymer bindings) -- as
well as to lighten util.js -- the escapeHTML function is moved into
`GrDiffBuilder`, its regex is made a constant, and is given a unit test.
Change-Id: I28c9f546cf50461e96995ecd6da8653e75554023
Diffs in PolyGerrit apply two shades of highlight to changed lines
(light and dark) to indicate the granularity of modifications and to
distinguish intraline edits. However, the logic for choosing the
background shade for diff lines would differ from that of GWT UI diffs
subtly.
+----------------------------------+----------------------------------+
| GWT UI Shading Logic | PG Shading Logic (incorrect) |
+----------------------------------+----------------------------------+
| Diff lines get a dark background | Diff lines get a dark background |
| IFF they appear in a delta chunk | IFF they do NOT contain any |
| that is empty on the left OR | intraline differences. |
| empty on the right. | |
+----------------------------------+----------------------------------+
| Diff lines get a light background otherwise. |
+---------------------------------------------------------------------+
With this change, the shading logic in PolyGerrit is modified to match
that of the GWT UI.
Bug: Issue 4219
Bug: Issue 5117
Change-Id: Ice24292df777118c08c3e73f771720f8a186a183
Empty diff lines had been coerced to non-breaking spaces in order to
preserve blank lines when copying to the clipboard [1]. Since then, copy
was upgraded to traverse the diff object rather than the DOM [2],
thereby obviating the need for non-breaking space to preserve blank
lines.
Remove the non-breaking space.
[1] 613b49c6
[2] 9fcec743
Bug: Issue 4854
Change-Id: I4039c76d996ed97e9fb237f2f6ca20f70be77675
Add a simple annotation layer that marks trailing whitespace in diffs
(guarded by the `show_whitespace_errors` diff preference). The newly
supported diff preference is added to both diff preference controls. The
requirement that all annotation layers must implement `addListener` is
relaxed as the trailing whitespace layer is the third layer that doesn't
use it.
Adds tests for the layer and the diff preference.
Feature: Issue 4836
Change-Id: Ifba05216bf0bc3c0a8a094f5ef392b983091d59f
Previously in Polygerrit, diff views were always displayed in the width
specified in diff preferences. This change gives the option to wrap
lines instead, which takes precedence over column width (the column
width option is hidden when line wrapping is selected), and fits the
diff view to screen.
The gerrit API already supports the 'lineWrapping' preference so this
change uses that already existing option.
Feature: Issue 4809
Change-Id: I0d9e292739b5910abfd04af63ec4c745bf06e446
+ These were slowing down tests in cases where it would actually hit a
live server, potentially adding the latency from the network to the
test.
+ Other fixes involve removing unused imports of util.js amongst other
small tweaks/fixes.
Bug: Issue 4016
Change-Id: I442deefebeffc6a701e4922faccfe1c74b3a35b6
Syntax highlighting is generally non-blocking, but on a line-by-line
basis. Thus, when large, minified JS or CSS libraries (which are
generally few very long lines) are included in a change, the syntax
highlighter blocks rendering and the browser slows to a halt.
With this change, the diff builder will disable syntax highlighting when
any line of the (processed) diff exceeds 499 characters in length. This
is a stopgap measure in lieu of making syntax non-blocking within lines.
Bug: Issue 4750
Change-Id: I777c7fb64cf02e21e99db0f1cbca4badd162b3c0
Previously, there was an issue where if you started scrolling on a diff
view page before everything was loaded, the page would jump back to the
top after loading finished.
This change temporarily adjusts scroll behavior when scrolling is
detected mid-diff load. The scroll behavior is then restored after
loading completes.
Bug: Issue 4411
Change-Id: I8175d433632fd8710f1353f7ba5635b826151ce0
