The diff cursor suspends the underlying cursor's scroll behavior while
the diff is rendering if the user scrolls the page before the render is
complete. However, when focus-on-move is enabled, the underlying cursor
also sets focus and this causes a scroll anyway. With this change the
cursor manager suspends the focus-on-move in the same way that it
suspends the scroll behavior to prevent this unwanted scroll/focus.
Bug: Issue 7390
Change-Id: I2bc7a06e940475b6b3c94995494f4207902acb40
Formerly, the gr-diff-view generated and updated the URL hash that
indicates the location of the diff cursor, regardless of the hash
generation scheme implemented in _generateUrl. With this change, the
diff view uses the _generateUrl for the cursor-location-specific URL.
Change-Id: Ia70a298f114c9eae9cdc5b0a8f73d7ecab55896e
Formerly, gr-diff would make its own REST calls for diff comments in the
patch range and path being viewed. However, these calls by the diff view
were redundant to the more general comment requests made by either
gr-diff-view (which requests all comments to support skipping to the
next file with comments) or gr-change-view (which requests comments
individually for each inline diff).
With this change, the diff component no longer loads comments for
itself, but is rather provided the comments from its host view via a
public property. In this way the host view can load the more-general
comment data, and filter it down for the diff view's params.
Introduces the gr-comment-api component to simplify loading and
filtering diff comments for use by diff views. By using this new
component:
* The diff view makes only one request for comments (including drafts
and robot comments) to support skipping to the next comment and
displaying comments in the diff view.
* The change view makes only one request for comments for all inline
diffs.
Bug: Issue 5299
Change-Id: Ia14a01619f1f9881aa0d253fd3f49af9a17f3dce
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
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
There was an issue where line numbers would not properly highlight in
inline-diffs rendered in the file view.
There were two separate issues that contributed to this problem:
1 - The diff cursor was not notified of the line being selected and
needing to move to that cursor stop.
2 - The diff cursor was not properly updated after being moved inside
a dom-if statement.
Additionally, the way that line selection was previously done was not
extendable to a diff cursor that stored multiple files. It queried
stops based on line number and side. This change also stores the
path of the file in the cursor stop so that the correct diff/line is
selected in the case of multiples.
Bug: Issue 5977
Change-Id: I7496293e19a4e59a3855dc78e273de6a9852e556
Previously, we did not scroll to target in the cursor manager if the top
was visible. However, there were times where the bottom content was
not visible, and it could have moved up into view.
This change passes an optional function for calcuating target height to
the cursor manager. If it is passed, the function is used to calculate
height instead of targetHeight. Height is ultimately used to determine
if the bottom is visible.
If the top is visible, but the bottom is not, scroll to the target if
the scroll position is farther down than the current position. Don't
scroll if the condition is not met because more content related to the
target is actually visible without scrolling, and do not want to reduce
it.
Bug: Issue 5498
Change-Id: I1708c921093b6e8b1916ae68fb468816f7c67633
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
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
The addition of incremental file loading in the file-list introduced an
edge case in which the user could scroll off the file-list with hotkeys.
This change implements the gr-cursor-manager to control file-list
scrolling behavior.
In addition, some interesting edge cases with the cursor-manager were
uncovered during use -- these were also fixed.
Change-Id: I936d4368058212c98684bc2617be2d98bdf6e41d
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
Moved the mock-diff-response test element to a more-appropriate location
and fixed two tests that were flaky in Safari.
Change-Id: I927973319b200021592b0c9e18c04a902f643894
Building on existing support for asynchronous diff processing, the
rendering stage is now also asynchronous. The `emitGroup` method of
gr-diff-builder (which constructs a DOM fragment and attaches it to the
document) is now called whenever the processor emits a new group, rather
than waiting for all groups to be available and looping over them.
Adds support for canceling the processor, and removes a behavior where
the diff was being re-rendered needlessly, causing visible flicker.
Updates a test that broke when rendering became asynchronous.
Change-Id: I37fcd65efc8c901b85fc93e91611c022edc10dc4
If the gr-diff-view is loaded with a line number address in the URL
hash, moves the diff cursor to that line.
Bug: Issue 4206
Change-Id: I359a29d7a3fd97b0ce72228aec3f49ae2295bfee
Sets the URL hash to the address of the cursor when a user selects a
diff line (either by clicking the line number or using the 'c' hotkey).
If the cursor is on a line of the revision, the address is the line
number. If the cursor is on a line of the base patch, the address is the
letter 'b' followed by the line number. Otherwise the address is the
empty string.
Bug: Issue 4206
Change-Id: Ic85da197eb6264ea1111cc34e781dccbc24d4d40
This change cleans up all lint errors reported by gjslint,
with the exception of third-party code in the gr-linked-text
element and everything under bower_components:
$ gjslint --custom_jsdoc_tags event --check_html \
-e bower_components,gr-linked-text -r app
Skipping 577 file(s).
181 files checked, no errors found.
Change-Id: I080d58bdd33b2d4b8dd22a717f06eebd7bbfb63d
Fixing UI data pipe line, re-rendering:
- store section DOM Elements in gr-diff-builder
- find diff section by line range
- store collapsed lines as gr-diff-group
- update gr-diff-builder's data model on expanding collapsed lines
- .right/.left CSS classes applied to td.lineNum instead of .content
Feature: Issue 3910
Change-Id: Ia0cb78aa00cfd910d186a37631fe71a89204a843
Reorganizes the keyboard shortcut list for change view with smaller
groups of shortcuts. Also tidies up the dialog for the diff view a bit.
Updates the style for the selected line in the diff view and fixes the
focus when the diff expand/collapse buttons are activated. The scroll
behavior is changed slightly.
Change-Id: I8e520f725f4706aaad6e8bd8b99dd0e274d2f830
Includes the gr-diff-cursor in the relevant views and wires up the
relevant keyboard commands to move the cursor around. Adds the new
keyboard commands to the keyboard shortcuts dialog.
Bug: Issue 4033
Change-Id: If9f63c11d28c7864c64ce8c9cfed6da4fcf4666a
Adds gr-diff-cursor, which is a specialized cursor for navigating
through diff content. It tracks the diff line being targeted, and, in
side-by-side mode, it tracks the side of the diff being targeted.
Also includes special behavior that ships blank spaces and other
non-commentable content in diffs, as well as automatically switching
sides when appropriate.
Support for more than one diff.
Also updates the diff builders to emit some useful element classes.
Note that this only adds the diff cursor along with tests. This is
setting up the work to actually add the diff cursor to the diff views.
Bug: Issue 4033
Change-Id: I991fd514f56ddd19d43d8e1354ad0d4fc71930c4
