Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat($location): allow automatic rewriting of links to be disabled #9487

Merged
merged 1 commit into from
Oct 8, 2014
Merged

feat($location): allow automatic rewriting of links to be disabled #9487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 27 additions & 17 deletions src/ng/location.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,8 @@ function $LocationProvider(){
var hashPrefix = '',
html5Mode = {
enabled: false,
requireBase: true
requireBase: true,
rewriteLinks: true
};

/**
Expand All @@ -648,15 +649,17 @@ function $LocationProvider(){
* @name $locationProvider#html5Mode
* @description
* @param {(boolean|Object)=} mode If boolean, sets `html5Mode.enabled` to value.
* If object, sets `enabled` and `requireBase` to respective values.
* - **enabled** – `{boolean}` – Sets `html5Mode.enabled`. If true, will rely on
* `history.pushState` to change urls where supported. Will fall back to hash-prefixed paths
* in browsers that do not support `pushState`.
* - **requireBase** - `{boolean}` - Sets `html5Mode.requireBase` (default: `true`). When
* html5Mode is enabled, specifies whether or not a <base> tag is required to be present. If
* `enabled` and `requireBase` are true, and a base tag is not present, an error will be
* thrown when `$location` is injected. See the
* {@link guide/$location $location guide for more information}
* If object, sets `enabled`, `requireBase` and `rewriteLinks` to respective values. Supported
* properties:
* - **enabled** – `{boolean}` – (default: false) If true, will rely on `history.pushState` to
* change urls where supported. Will fall back to hash-prefixed paths in browsers that do not
* support `pushState`.
* - **requireBase** - `{boolean}` - (default: `true`) When html5Mode is enabled, specifies
* whether or not a <base> tag is required to be present. If `enabled` and `requireBase` are
* true, and a base tag is not present, an error will be thrown when `$location` is injected.
* See the {@link guide/$location $location guide for more information}
* - **rewriteLinks** - `{boolean}` - (default: `false`) When html5Mode is enabled, disables
* url rewriting for relative linksTurns off url rewriting for relative links.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description of rewriteLinks needs...rewriting.

For one, it is true by default.
Besides the obvious left over sentence at the end, I think that saying it disables url rewriting is not accurate, since it depends on the value. How about:

When html5Mode is enabled, it enables/disables url rewriting for relative links.

*
* @returns {Object} html5Mode object if used as getter or itself (chaining) if used as setter
*/
Expand All @@ -665,12 +668,19 @@ function $LocationProvider(){
html5Mode.enabled = mode;
return this;
} else if (isObject(mode)) {
html5Mode.enabled = isBoolean(mode.enabled) ?
mode.enabled :
html5Mode.enabled;
html5Mode.requireBase = isBoolean(mode.requireBase) ?
mode.requireBase :
html5Mode.requireBase;

if (isBoolean(mode.enabled)) {
html5Mode.enabled = mode.enabled;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the extra space after = ?

}

if (isBoolean(mode.requireBase)) {
html5Mode.requireBase = mode.requireBase;
}

if (isBoolean(mode.rewriteLinks)) {
html5Mode.rewriteLinks = mode.rewriteLinks;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidental typo. Would you mind sending a pr to fix it?
On Oct 8, 2014 3:47 PM, "Georgios Kalpakas" notifications@github.com
wrote:

In src/ng/location.js:

  •      mode.enabled :

  •      html5Mode.enabled;

  •  html5Mode.requireBase = isBoolean(mode.requireBase) ?

  •      mode.requireBase :

  •      html5Mode.requireBase;

  •  if (isBoolean(mode.enabled)) {

  •    html5Mode.enabled =  mode.enabled;

  •  }

  •  if (isBoolean(mode.requireBase)) {

  •    html5Mode.requireBase = mode.requireBase;

  •  }

  •  if (isBoolean(mode.rewriteLinks)) {

  •    html5Mode.rewriteLinks =  mode.rewriteLinks;

Same as above.

—
Reply to this email directly or view it on GitHub
https://github.com/angular/angular.js/pull/9487/files#r18617846.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IgorMinar : #9520 (I also added the docs fix).

}

return this;
} else {
return html5Mode;
Expand Down Expand Up @@ -763,7 +773,7 @@ function $LocationProvider(){
// TODO(vojta): rewrite link when opening in new tab/window (in legacy browser)
// currently we open nice url link and redirect then

if (event.ctrlKey || event.metaKey || event.which == 2) return;
if (!html5Mode.rewriteLinks || event.ctrlKey || event.metaKey || event.which == 2) return;

var elm = jqLite(event.target);

Expand Down
47 changes: 27 additions & 20 deletions test/ng/locationSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,19 @@ describe('$location', function() {
});


it ('should not rewrite links when rewriting links is disabled', function() {
configureService('/a?b=c', true, true, '', 'some content', false);
inject(
initBrowser(),
initLocation(),
function($browser) {
browserTrigger(link, 'click');
expectNoRewrite($browser);
}
);
});


it('should rewrite full url links to same domain and base path', function() {
configureService({linkHref: 'http://host.com/base/new', html5Mode: true});
inject(
Expand Down Expand Up @@ -1793,11 +1806,13 @@ describe('$location', function() {


describe('html5Mode', function() {
it('should set enabled and requireBase when called with object', function() {
it('should set enabled, requireBase and rewriteLinks when called with object', function() {
module(function($locationProvider) {
$locationProvider.html5Mode({enabled: true, requireBase: false, rewriteLinks: false});
expect($locationProvider.html5Mode()).toEqual({
enabled: false,
requireBase: true
enabled: true,
requireBase: false,
rewriteLinks: false
});
});

Expand All @@ -1809,12 +1824,14 @@ describe('$location', function() {
module(function($locationProvider) {
$locationProvider.html5Mode({
enabled: 'duh',
requireBase: 'probably'
requireBase: 'probably',
rewriteLinks: 'nope'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since requireBase and rewriteLinks are by default true, it might make the test more explicit to assign falsy (but non-boolean) values, in order to showcase that they are indeed not updated. E.g. something like:

        $locationProvider.html5Mode({
          enabled: 'duh',
          requireBase: '',
          rewriteLinks: 0
        });

});

expect($locationProvider.html5Mode()).toEqual({
enabled: false,
requireBase: true
requireBase: true,
rewriteLinks: true
});
});

Expand All @@ -1830,31 +1847,21 @@ describe('$location', function() {

expect($locationProvider.html5Mode()).toEqual({
enabled: false,
requireBase: true
});
});

inject(function(){});
});


it('should default to enabled:false and requireBase:true', function() {
module(function($locationProvider) {
expect($locationProvider.html5Mode()).toEqual({
enabled: false,
requireBase: true
requireBase: true,
rewriteLinks: true
});
});

inject(function(){});
});


it('should return html5Mode object when called without value', function() {
it('should default to enabled:false, requireBase:true and rewriteLinks:true', function() {
module(function($locationProvider) {
expect($locationProvider.html5Mode()).toEqual({
enabled: false,
requireBase: true
requireBase: true,
rewriteLinks: true
});
});

Expand Down