feat(location): add configurable attr to force html5mode link rewriting #14976

onlywei · 2016-08-01T23:22:22Z

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature

What is the current behavior? (You can also link to an open issue here)
When rewriteLinks is disabled in Location's html5mode, all links are treated as if they should reload from the server.

Also see this discussion: #14959

What is the new behavior (if this is a feature change)?
This allows the user to add an ng-noserver attribute to the <a> tag, like this:
<a href="somewhere" ng-noserver>click me</a> to explicitly mark this link as one that should not reload from server, i.e. to use angular javascript routing instead.

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

Other information:

See this discussion: #14959

When using html5mode, sometimes it is necessary to not use the tag
in order to support SVG icons. An example of this is in the discussion
linked above.

When we do this, sometimes we also want to disable "rewriteLinks" so that
we can still leave the Angular application via navigation.

This feature allows the user to explicitly mark an tag as a link that
should not refresh from server, even if rewriteLinks is disabled.

gkalpak · 2016-08-05T11:26:57Z

src/ng/location.js

@@ -877,6 +879,8 @@ function $LocationProvider() {
        if (elm[0] === $rootElement[0] || !(elm = elm.parent())[0]) return;
      }

+      if (!html5Mode.rewriteLinks && isUndefined(elm.attr('ng-noserver'))) return;


This will not recognize prefixed forms (such as x-ng-... or data-ng-...) - and I think this is undesirable.

gkalpak · 2016-08-05T11:34:49Z

I think noserver is not a very intuitive name. Also URL-rewriting doesn't guarantee that no request will be done to the server afaict.

onlywei · 2016-08-05T16:28:56Z

Thanks @gkalpak, I was hoping to get this kind of useful feedback. I've updated the code. Please review at your leisure.

gkalpak · 2016-08-07T09:00:41Z

src/ng/location.js

+   *   - **rewriteLinks** - `{boolean|String}` - (default: `true`) When html5Mode is enabled,
+   *     enables/disables url rewriting for relative links. If set to a string, url rewriting will
+   *     only happen on links with an attribute that matches the given string. For example, if set
+   *     to "internalLink", then the URL will only be rewritten for `<a internal-link>` links.


There is no attribute name (de)normalization going on. In order to match <a internal-link> you need to set rewriteLinks to 'internal-link' (not 'internalLink').

gkalpak · 2016-08-07T13:14:14Z

Btw, docs need to be updated in other places (e.g. the Using $location section of the developer guide).
Adding it here as a reminder, but feel free to defer that until the implementation has finalized.

gkalpak · 2016-08-07T17:23:22Z

test/ng/locationSpec.js

        module(function($locationProvider) {
          $locationProvider.html5Mode({
            enabled: 'duh',
            requireBase: 'probably',
-            rewriteLinks: 'nope'
+            rewriteLinks: false


This change doesn't test what it should. You need to supply an unsupported type and verify that it didn't get copied to the actual config.

See this discussion: angular#14959 When using html5mode, sometimes it is necessary to not use the <base> tag in order to support SVG icons. An example of this is in the discussion linked above. When we do this, sometimes we also want to disable "rewriteLinks" so that we can still leave the Angular application via navigation. This feature allows the user to explicitly mark an <a> tag as a link that should not refresh from server, even if rewriteLinks is disabled.

gkalpak · 2016-08-08T13:07:33Z

LGTM

gkalpak · 2016-08-08T13:08:22Z

I'll let @petebacondarwin do the final LGTM, since he is the $location master 😃

onlywei · 2016-08-19T21:03:53Z

@petebacondarwin Ping. Please review when convenient.

petebacondarwin · 2016-08-20T19:24:32Z

LGTM

… attribute In HTML5 mode, links can now be selectively rewritten, by setting `mode.rewriteLinks` to a string (denoting an attribute name). Anchor elements that have the specified attribute will be rewritten, while other links will remain untouched. This can be useful in situations where it is desirable to use HTML5 mode without a `<base>` tag, but still support rewriting specific links only. See #14959 for more details on a possible usecase. Closes #14976

… attribute In HTML5 mode, links can now be selectively rewritten, by setting `mode.rewriteLinks` to a string (denoting an attribute name). Anchor elements that have the specified attribute will be rewritten, while other links will remain untouched. This can be useful in situations where it is desirable to use HTML5 mode without a `<base>` tag, but still support rewriting specific links only. See angular#14959 for more details on a possible usecase. Closes angular#14976

… attribute In HTML5 mode, links can now be selectively rewritten, by setting `mode.rewriteLinks` to a string (denoting an attribute name). Anchor elements that have the specified attribute will be rewritten, while other links will remain untouched. This can be useful in situations where it is desirable to use HTML5 mode without a `<base>` tag, but still support rewriting specific links only. See #14959 for more details on a possible usecase. Closes #14976

googlebot added the cla: yes label Aug 1, 2016

gkalpak reviewed Aug 5, 2016
View reviewed changes

gkalpak added type: feature frequency: low component: $location severity: inconvenient labels Aug 5, 2016

gkalpak added this to the Backlog milestone Aug 5, 2016

onlywei force-pushed the force-link-rewrite branch from 87a1dad to 36e7a6d Compare August 5, 2016 16:28

gkalpak reviewed Aug 7, 2016
View reviewed changes

gkalpak changed the title ~~feat(location): add ng-noserver attr to force html5mode link rewriting~~ feat(location): add configurable attr to force html5mode link rewriting Aug 7, 2016

onlywei force-pushed the force-link-rewrite branch from 36e7a6d to 368d9bf Compare August 7, 2016 16:08

gkalpak reviewed Aug 7, 2016
View reviewed changes

onlywei force-pushed the force-link-rewrite branch from 368d9bf to 5eae33d Compare August 7, 2016 19:06

gkalpak closed this in 3d686a9 Aug 22, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(location): add configurable attr to force html5mode link rewriting #14976

feat(location): add configurable attr to force html5mode link rewriting #14976

onlywei commented Aug 1, 2016

gkalpak Aug 5, 2016 •

edited

Loading

gkalpak commented Aug 5, 2016

onlywei commented Aug 5, 2016

gkalpak Aug 7, 2016

onlywei Aug 7, 2016

gkalpak commented Aug 7, 2016

gkalpak Aug 7, 2016

onlywei Aug 7, 2016

gkalpak commented Aug 8, 2016

gkalpak commented Aug 8, 2016

onlywei commented Aug 19, 2016

petebacondarwin commented Aug 20, 2016

feat(location): add configurable attr to force html5mode link rewriting #14976

feat(location): add configurable attr to force html5mode link rewriting #14976

Conversation

onlywei commented Aug 1, 2016

gkalpak Aug 5, 2016 • edited Loading

Choose a reason for hiding this comment

gkalpak commented Aug 5, 2016

onlywei commented Aug 5, 2016

gkalpak Aug 7, 2016

Choose a reason for hiding this comment

onlywei Aug 7, 2016

Choose a reason for hiding this comment

gkalpak commented Aug 7, 2016

gkalpak Aug 7, 2016

Choose a reason for hiding this comment

onlywei Aug 7, 2016

Choose a reason for hiding this comment

gkalpak commented Aug 8, 2016

gkalpak commented Aug 8, 2016

onlywei commented Aug 19, 2016

petebacondarwin commented Aug 20, 2016

gkalpak Aug 5, 2016 •

edited

Loading