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

Conversation

IgorMinar
Copy link
Contributor

Currently, when the location provider is set to html5 mode, all links
on the page are hijacked and automatically rewritten. While this may be
desirable behavior in some cases (such as using ngRoute), not all cases
where html5 mode are enabled imply the desire for this behavior.

One example would be an application using the
ui-router library, with some
pages that exist outside of angular. Links that are meant to go through
the router use the ui-sref directive, so the rewrite behavior is
unnecessary.

Closes #5487

@IgorMinar IgorMinar added this to the 1.3.0-rc.5 milestone Oct 8, 2014
@IgorMinar
Copy link
Contributor Author

@tbosch lgty?

});


it('should default to enabled:false and requireBase:true', function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this test because it's the same test as the one above.

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@@ -657,6 +658,8 @@ function $LocationProvider(){
* `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}` - Turns off url rewriting for relative links (default:
* `false`).
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this rather be something like:

  *   - **rewriteLinks** - `{boolean}` - Sets `html5Mode.rewriteLinks` (default: `true`). When
  *     html5Mode is enabled, it enables/disables url rewriting for relative links.

Also, on line 652 above:

   *   If object, sets `enabled`, `requireBase` and `rewriteLinks` to respective values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually think that "html5Mode.rewriteLinks" is meaning less to the user. it's an implementation detail that is not visible to the user unless they read the source code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rewrite these paragraphs

@tbosch
Copy link
Contributor

tbosch commented Oct 8, 2014

LGTM, agree with the comments by @gkalpak.

@tbosch tbosch assigned IgorMinar and unassigned tbosch Oct 8, 2014
@jeffbcross jeffbcross force-pushed the master branch 2 times, most recently from abdaab7 to 30996f8 Compare October 8, 2014 19:47
Currently, when the location provider is set to html5 mode, all links
on the page are hijacked and automatically rewritten. While this may be
desirable behavior in some cases (such as using ngRoute), not all cases
where html5 mode are enabled imply the desire for this behavior.

One example would be an application using the
[ui-router](https://github.com/angular-ui/ui-router) library, with some
pages that exist outside of angular. Links that are meant to go through
the router use the `ui-sref` directive, so the rewrite behavior is
unnecessary.

Closes angular#5487
@IgorMinar IgorMinar added cla: yes and removed cla: no labels Oct 8, 2014
@IgorMinar IgorMinar merged commit b3e09be into angular:master Oct 8, 2014
@btford btford removed the In Progress label Oct 8, 2014
* 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.

@gkalpak gkalpak mentioned this pull request Oct 9, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants