Pagination directive, IE11 issue with prev/next buttons

[scolt]

Bug description:

After click on disabled previous/next button application redirects user to home page.

Steps to reproduce via plunker below

1. Open home page in IE11 (Internet Explorer 11)
2. Click on "Pagination" link
3. Observe that previous button is disabled
4. Click on previous button

Actual result:
Home page is opened.

Expected result:
Nothing should happen.

Link to minimally-working plunker that reproduces the issue:

http://plnkr.co/edit/1QBjhagSpwAb5dvFtm6S?p=preview

Version of Angular, UIBS, and Bootstrap

Angular: 1.4.7

UIBS: 2.1.4

Bootstrap: 3.3.3

Recommendation for fixes

As I see, issue occurred because "href" is provided. This is empty attribute and in results it can be removed without any regression.

[roger2hk]

I am having the same issue and was about to create one.
The same issue happens in pager as well.
The fix is to remove those href in uib/template/pagination/pagination.html and uib/template/pager/pager.html.

I will now create a pull request.

[roger2hk]

I just realize removing the empty href would break the CSS. My current workaround is to change the empty href to href="/".

Here is the Plunker that fixed the issue by using href="/".
http://plnkr.co/edit/6shfJl7yL5OcURpMSuo8?p=preview

[wesleycho]

I need to verify my hypothesis at work tomorrow on a Windows VM with IE11, but I suspect this is a usage problem - this demo is not quite a minimal reproduction indicating it is an issue specifically with this library as it is polluted with ngRoute, and I have strong suspicions that that is actually the cause of this issue.

Please slim down the reproduction to not contain ngRoute if possible - otherwise, this is highly likely not a valid library issue, and thus filed in the wrong repository.

[roger2hk]

The same issue happens in ui-router as well. Hope my information helps.

[roger2hk]

@scolt There is another workaround before this issue is fixed.

According to the issue raised in ui-router, the workaround is to add a customized CSS. However, this would break the cursor: not-allowed; impacting the user experience.
angular-ui/ui-router#2957

.pagination > .disabled > a {
  pointer-events: none;
}

[wesleycho]

Created a minimal reproduction from the homepage here - the Plunker in the OP had other distractions/issues with it that would prevent proper fixing.

[wesleycho]

Hm, I was able to reproduce this on the homepage itself, but looks like my reproduction doesn't illustrate this...there's something else going on here.

[wesleycho]

I'm not sure I will have the time to give this issue the full investigation it needs, but here are some important aspects that any fix around this area must consider. For some reason it appears IE11 is not trigging a click on the anchor element - I suspect the ng-disabled is breaking it, but removing it harms accessibility significantly. The selectPage method isn't being called, which is what executes event.preventDefault(). In addition, any solution must consider accessibility and must not commit a CSP violation.

I suspect there is going to be no good solution, because touching this likely will cause regressions on one of the other points. Overriding the template and removing the href might be the best solution, but even that harms accessibility and I believe is invalid HTML.

[roger2hk]

In the Bootstrap component document, only li element requires the disabled class. It should be safe to remove the ng-disabled in a element.

<nav aria-label="...">
  <ul class="pagination">
    <li class="disabled"><a href="#" aria-label="Previous"><span aria-hidden="true">&laquo;</span></a></li>
    <li class="active"><a href="#">1 <span class="sr-only">(current)</span></a></li>
    ...
  </ul>
</nav>

According to W3C HTML5, there is no disabled attribute for the a element.

I have forked your Plunker with the proposed fix to remove ng-disabled in a element and the result looks good.
https://plnkr.co/edit/Nltv1MPUy9lT54OJxFmX?p=preview

[roger2hk]

@wesleycho If you think removing the ng-disabled in a element is a good solution, I can help create the pull request.

[wesleycho]

Removing it will make the element tabable when it is disabled, which is an accessibility violation/regression.

[roger2hk]

@wesleycho How about removing the uib-tabindex-toggle directive and make use of ng-attr-tabindex?
This should have avoided the accessibility violation.

<li ng-if="::boundaryLinks" ng-class="{disabled: noPrevious()||ngDisabled}" class="pagination-first"><a href ng-click="selectPage(1, $event)" ng-attr-tabindex="{{(noPrevious()||ngDisabled) && '-1' || undefined}}">{{::getText('first')}}</a></li>
<li ng-if="::directionLinks" ng-class="{disabled: noPrevious()||ngDisabled}" class="pagination-prev"><a href ng-click="selectPage(page - 1, $event)" ng-attr-tabindex="{{(noPrevious()||ngDisabled) && '-1' || undefined}}">{{::getText('previous')}}</a></li>
<li ng-repeat="page in pages track by $index" ng-class="{active: page.active,disabled: ngDisabled&&!page.active}" class="pagination-page"><a href ng-click="selectPage(page.number, $event)" ng-attr-tabindex="{{(ngDisabled&&!page.active) && '-1' || undefined}}">{{page.text}}</a></li>
<li ng-if="::directionLinks" ng-class="{disabled: noNext()||ngDisabled}" class="pagination-next"><a href ng-click="selectPage(page + 1, $event)" ng-attr-tabindex="{{(noNext()||ngDisabled) && '-1' || undefined}}">{{::getText('next')}}</a></li>
<li ng-if="::boundaryLinks" ng-class="{disabled: noNext()||ngDisabled}" class="pagination-last"><a href ng-click="selectPage(totalPages, $event)" ng-attr-tabindex="{{(noNext()||ngDisabled) && '-1' || undefined}}">{{::getText('last')}}</a></li>

https://plnkr.co/edit/bLPJL88GCB6xGuxjXmp1?p=preview

[roger2hk]

@wesleycho Do you think this is a good solution which has considered the accessibility? Making use of the uib-tabindex-toggle directive sounds a much better approach. I want to make sure the direction of proposed solution is correct.

[hieuxlu]

Is it possible to replace a href with a href="javascript:void(0)" in default Angular UI pagination template? The issue still has not been fixed yet.

[roger2hk]

Using javascript:void(0) would cause the Content Security Policy (CSP) violation. This has already been discussed in #3904.

[hieuxlu]

@roger2hk Thanks. For now, removing ng-disabled in link element seems like a good enough workaround for me.

[fflow]

@wesleycho is this bug activly beening worked on? What is the current status oft it? Or did you move it to the end of your backlog :) ?

[corner22]

The fix I have successfully used is to remove the ng-disabled and uib-tabindex-toggle attributes from the anchor elements and replace with an ng-attr-tabindex that sets tabindex to '0' if not disabled and '-1' if disabled:

<li ng-if="::boundaryLinks" ng-class="{disabled: noPrevious()||ngDisabled}" class="pagination-first"><a href ng-click="selectPage(1, $event)" ng-attr-tabindex="{{ (noPrevious()||ngDisabled) ? '-1' : '0' }}">{{::getText('first')}}</a></li>
<li ng-if="::directionLinks" ng-class="{disabled: noPrevious()||ngDisabled}" class="pagination-prev"><a href ng-click="selectPage(page - 1, $event)" ng-attr-tabindex="{{ (noPrevious()||ngDisabled) ? '-1' : '0' }}">{{::getText('previous')}}</a></li>
<li ng-repeat="page in pages track by $index" ng-class="{active: page.active,disabled: ngDisabled&&!page.active}" class="pagination-page"><a href ng-click="selectPage(page.number, $event)" ng-attr-tabindex="{{ (ngDisabled&&!page.active) ? '-1' : '0' }}">{{page.text}}</a></li>
<li ng-if="::directionLinks" ng-class="{disabled: noNext()||ngDisabled}" class="pagination-next"><a href ng-click="selectPage(page + 1, $event)" ng-attr-tabindex="{{ (noNext()||ngDisabled) ? '-1' : '0' }}">{{::getText('next')}}</a></li>
<li ng-if="::boundaryLinks" ng-class="{disabled: noNext()||ngDisabled}" class="pagination-last"><a href ng-click="selectPage(totalPages, $event)" ng-attr-tabindex="{{ (noNext()||ngDisabled) ? '-1' : '0' }}">{{::getText('last')}}</a></li>

[roger2hk]

@corner22 I am glad that you have a similar proposal as mine #6262 (comment).

I will create a PR in the coming few days. Hope someone from the team will accept it.

[dadoadk]

I'm having this same issue. IE11 is still used widely in enterprise, so for this iteration I'll use cursor: not-allowed;. When is the next version expected?

[roger2hk]

@dadoadk Pull request has been created but there is no active team member to review and approve it. I would say there is no ETA.

[warjohn123]

Hi, any updates on this?

[jamespsterling]

+1, just found this bug. Any progress on a fix?

My temporary workaround is just adding href='#' to the pagination links:

angular.module("uib/template/pagination/pagination.html", []).run(["$templateCache", function($templateCache) {
  $templateCache.put("uib/template/pagination/pagination.html",
    "<li role=\"menuitem\" ng-if=\"::boundaryLinks\" ng-class=\"{disabled: noPrevious()||ngDisabled}\" class=\"pagination-first\"><a href='#' ng-click=\"selectPage(1, $event)\" ng-disabled=\"noPrevious()||ngDisabled\" uib-tabindex-toggle>{{::getText('first')}}</a></li>\n" +
    "<li role=\"menuitem\" ng-if=\"::directionLinks\" ng-class=\"{disabled: noPrevious()||ngDisabled}\" class=\"pagination-prev\"><a href='#' ng-click=\"selectPage(page - 1, $event)\" ng-disabled=\"noPrevious()||ngDisabled\" uib-tabindex-toggle>{{::getText('previous')}}</a></li>\n" +
    "<li role=\"menuitem\" ng-repeat=\"page in pages track by $index\" ng-class=\"{active: page.active,disabled: ngDisabled&&!page.active}\" class=\"pagination-page\"><a href ng-click=\"selectPage(page.number, $event)\" ng-disabled=\"ngDisabled&&!page.active\" uib-tabindex-toggle>{{page.text}}</a></li>\n" +
    "<li role=\"menuitem\" ng-if=\"::directionLinks\" ng-class=\"{disabled: noNext()||ngDisabled}\" class=\"pagination-next\"><a href='#' ng-click=\"selectPage(page + 1, $event)\" ng-disabled=\"noNext()||ngDisabled\" uib-tabindex-toggle>{{::getText('next')}}</a></li>\n" +
    "<li role=\"menuitem\" ng-if=\"::boundaryLinks\" ng-class=\"{disabled: noNext()||ngDisabled}\" class=\"pagination-last\"><a href='#' ng-click=\"selectPage(totalPages, $event)\" ng-disabled=\"noNext()||ngDisabled\" uib-tabindex-toggle>{{::getText('last')}}</a></li>\n" +
    "");
}]);

[roger2hk]

@jamespsterling This project is no longer being maintained.