This also moves the change reloading logic back to the change view,
where it gets updated patch ranges via a two-way data binding.
Change-Id: Ib09ad1a176ba96bac77a513d344226df029aef7b
The gr-diff-cursor#getAddress method returns null when it has no
position or no address corresponds to it's position. However, the
gr-diff-view#_onLineSelected method did not account for this case and
attempted to use null addresses to construct URLs when moving the cursor
to a "File" line of a diff.
With this change, the diff view neither uses line number nor the diff
side when constructing URLs if the cursor does not yield an address.
Change-Id: I628658295bca1f49e0c2d3484e2e0d01e71bcd91
Slight refactoring required in order to satisfy both use cases:
- Fire an event when patch range changes and let parent element
handle it.
- Support comment strings
- availablePatches becomes an array of objects instead of an
array of integers.
Change-Id: Ia8da9296f41eb2d45c9358d03fbec3940273ad9d
Some URLs (for example, comment links in Gerrit emails) specify the line
number following an @-sign rather than in the hash using an octothorp
because the URL is already mostly inside a hash (for backward
compatibility with the GWT UI). When these URLs do not include the
project name (as they currently do not in Gerrit emails) the project is
loaded and the parsed route is "upgraded" to include the project.
However, when generating the upgrade URL, the `_generateUrl` method
looks for the `lineNum` and `leftSide` properties to generate the
address. While the address specified by the @-sign is properly converted
to an octothorp-hash before this point, it appears on the properties
object as `hash`, and is thus ignored by `_generateUrl`.
With this change, instead of passing the raw hash through the app params
and parsing it in the `gr-diff-view`, the hash is parsed into its
`leftSide` and `lineNum` values and attached to the `app.params` by the
router. In this way, the location is preserved through URL upgrade, the
param format used in navigation matches that used in URL generation and
the diff view no longer parses the route.
Bug: Issue 7087
Change-Id: Idb2e3cccf2884fae742247cf1ebbde1ad97e53ab
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
To prevent redundant (and sometimes incorrect) change/diff loads, this
change moves the logic for rewriting the URL schema to within the
router.
This has the drawback of adding a step -- and a potential API call --
between initial URL interpretation and first render, but ensures that
render will be correct. This only occurs when legacy routes are used.
Bug: Issue 7027
Change-Id: Ia1571e2f946f6d9e334626b6b0fd4a8c0c81f35d
Modifying the reviewed state of an edit patchset is an illogical action,
and should be disabled.
This change adds an _editLoaded property to gr-diff-view to
conditionally hide the reviewed checkbox. In addition, attempts to
modify the reviewed state are treated as a no-op.
TODO: Further utilize this approach to conditionally show buttons for
modifying an edit patchset. Will be done in a later change.
Bug: Issue 4437
Change-Id: I0ea98e49c5ec66e73f45fef9db5e3849d6e594df
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
The migration to project-based URLs has made evident the need for some
alias in gr-navigation to `history.replaceState`. This change calls
the function `upgradeUrl` within gr-diff-view and gr-change-view iff the
project returned by getChangeDetail differs from the one set on
app.params.
Bug: Issue 6708
Change-Id: I7b542ddb989527b6a89ccdfd79846808835ed5a3
Previously, the app relied on string matching of the view tag names in
order to perform view-based actions (e.g. routing).
This change refactors those instances to utilise the Gerrit.Nav.View
enum.
Bug: Issue 6708
Change-Id: If0212fde93e0167e3207af19006beee1a602df60
Previously, there was an issue where nagivating between diffs would
use the hash from the previous diff because the url had not been
updated when _paramsChanged was calls. Instead of getting the hash
from window.location, let the router handle it and pass it in as a
param.
Bug: Issue 6702
Change-Id: I03cb59e0f55813b0973eaf949e9e0958946d094e
Split the path at the last slash to provide only the basename as the
page title in diff-view.
Bug: Issue 4955
Change-Id: I242d38f5823415a53f837ad7d1a5b97bd16c6fa6
Polymer2 does not support type extension. As such, elements that rely on
it are updated.
Instead of
<select is="gr-select"><select>
This will now be..
<gr-select>
<select></select>
</gr-select>
This is similar to the implementation of iron-input, except that is more
complex, as it is supporting both type extention and non-type extension.
https://github.com/PolymerElements/iron-input/blob/master/iron-input.html
Bug: Issue 6371
Change-Id: I31091ff24791a9dc073b3325c7b0daa1580b69ef
This allows for the functions which rely on the properties to be called when
the properties are set.
Change-Id: I306550a246c4c535ae5f165ce3fbe5282eea25f2
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
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
Previously, gr-diff-preferences was the contents that was displayed in
an overlay contained in the gr-diff-view. There were a handful of
functions that crossed between the two-- the diff preferences element
would fire events that needed to be handled by the diff view.
Because the gr-diff-preferences element will be added to the change
view as well, the overlay has been moved to be part of the diff
preferences element. This way, the element can handle all of the actions
taken by the panel, and all the parent element needs to do is call
the open function.
A separate change will come with the addition of diff preferences in the
change view.
Bug: Issue 5426
Change-Id: Id7396147e73354122ea3825bde2c324b5daa1d26
GWT UI has this button but polygerrit dosen't. Makes it easier to view
your other diff's if you need to pick another file but it is way down
the list.
Change-Id: Ic62480315b3b00d7623d4ac444a0b554d255b4d1
Adds keyboard shortcuts to the diff view to navigate to the next or
previous file in the change's file list that has comments in the current
patch range.
Feature: Issue 5235
Change-Id: I1ad39089c1ac227e335093f25b72311f7e98b3f7
If a user in diff view changes the diff range to one that no longer
includes the file that they are on, the Prev/Next links are grayed out
and they are forced to return to the change view to navigate the diff.
This change sets Next to direct to the first file in the list and Prev
to point to the last file in the list.
Bug: Issue 4932
Change-Id: Ifb460c9721bfafbc19afa68253402b9dcd2f2c3e
Previously, there was logic regarding diff view defaults in both the
diff view and also the file list views. When the unified view became
the mobile default for the diff view, the file list was forgotten, and
if a user visited a change view first and then a diff view (without
refreshing the page) they wouldn't get defaulted to unified on mobile.
This change fixes the issue and moves the logic for which view type to
display to the rest interface, so that it doesn't have to be implemented
in multiple places.
Bug: Issue 5119
Change-Id: I95bfe1540cc9439bd6d3e3e39d13a5e32962b7fa
Previously, the file path was truncated and often the file name was cut
off completely, which had made it hard to tell what files were actually
changed. With this change, the text appearing on the file list just
show ellipsis and the file name (ex: '.../filename.txt').
Additionally, for both mobile and larger screens, the full filename
appears (line wrapped if needed) when the file list item is expanded.
This way, if enough content is cut off that it's still not useful, there
is a way to see the path in it's entirety.
Bug: Issue 4609
Change-Id: Ic4aaf45bafbc3c5b31add8f7c43b18c9d2b2913b
+ 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
Previously, the line marker was only subtly visible by the highlighted
line number. This change adds a bottom border to the selected line if
the user is using keycodes (j, k, up, down) to more the cursor. When
the escape key is pressed, the distinguished line marker will dissapear.
Feature: Issue 4739
Change-Id: If8c751efc137ef87cfdad1c8bf7d905de1219107
A dependent change added the ability to download a single file diff to
Gerrit. This change utilizes that new feature via a download link in the
diff-view.
Feature: Issue 4669
Change-Id: I87ef2324ff2cd7fab6eb4b2e066dd08defe7c4f0
When clicking a line number in the diff view, set the URL hash using
`history.replaceState` rather than `history.pushState` to avoid an
additional history entry.
Bug: Issue 4820
Change-Id: If2101508a49ac15e955d2981f7c7f93f22d5b9f9
PolyGerrit uses the format b<line number> in URLs to indicate a specific
line number in the base patch, but the GWT UI would use the format of
a<line number>. Update PolyGerrit to understand both.
Bug: Issue 4792
Change-Id: I8d09be9cb952c66e085d3a4777c04e6d79eab518
Previously, long file names would display outside of the window size,
causing a horizontal scroll on mobile browsers.
With this change:
* In the file list on the change view, the rightControls are adjusted so
that they wrap on a new line on smaller/mobile screens.
* In the comment list, word-wrapping is used for long file paths
displayed above each comment.
* In the diff view, instead of displaying full paths in the dropdown,
the path is truncated to include an ellipsis, a slash and the file name
so that it can be seen both in the dropdown and also the native browser
select.
Bug: Issue 4722
Change-Id: Icd4644a45db71bc6666c21d62c864d91e9874654
Previously, the default view type was displayed for all screen sizes.
If side by side was the user's preference, and they viewed polygerrit
on mobile, side by side was displayed.
This change ignores user preference for small screen sizes (under 900
px wide). The user can still toggle to a split view, but unified will
be the first view shown.
Bug: Issue 4682
Change-Id: Id4e1cab17de433033e103c3cca582b7e9c656acf
Setting persists for the duration of viewing a change. When a user
selects a new change, their default view mode is set.
Bug: Issue 4670
Change-Id: I4eade0e42c13bf2b3079ef38fb07239f98a243d8
In some cases, the "press c to comment" feature was overriding default
browser copy behavior. This change checks for modifiers before creating
comments. This functionality is factored out into a function for future
use.
Bug: Issue 3989
Change-Id: I3ebe0dec2f5436b8339d81d99d1287799ff3568b
Previously, the diff preferences form did not automatically focus to the
first textfield and did not allow tabbing between input fields. This
change adds autofocus when the overlay is opened and allows for tabbing
between the other input fields in the modal.
Bug: Issue 4140
Change-Id: If15812bb4404ca4061597755eeaf68d4cae23b3f
When gr-diff-view attaches, it may need the user's preferences in order
to set the diff mode selector to the correct state.
In the course of testing this change, I realized that an overriding
diff mode selection wasn't sticking across pages, especially when moving
back and forth in history. Modified gr-change-view to also update the
diff mode from the user's preferences when it notices a different change
is being loaded.
Bug: Issue 4434
Change-Id: Id2041b55bf66d7de66f74d7765ed0db717caeebf
