Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add role="dialog" in modals via JavaScript #30936

Merged
merged 3 commits into from
Jun 4, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 2 additions & 0 deletions js/src/modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ class Modal {
this._element.style.display = 'block'
this._element.removeAttribute('aria-hidden')
this._element.setAttribute('aria-modal', true)
this._element.setAttribute('role', 'dialog')
this._element.scrollTop = 0

if (modalBody) {
Expand Down Expand Up @@ -323,6 +324,7 @@ class Modal {
this._element.style.display = 'none'
this._element.setAttribute('aria-hidden', true)
this._element.removeAttribute('aria-modal')
this._element.removeAttribute('role')
this._isTransitioning = false
this._showBackdrop(() => {
document.body.classList.remove(CLASS_NAME_OPEN)
Expand Down
6 changes: 6 additions & 0 deletions js/tests/unit/modal.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ describe('Modal', () => {

modalEl.addEventListener('shown.bs.modal', () => {
expect(modalEl.getAttribute('aria-modal')).toEqual('true')
expect(modalEl.getAttribute('role')).toEqual('dialog')
expect(modalEl.getAttribute('aria-hidden')).toEqual(null)
expect(modalEl.style.display).toEqual('block')
expect(document.querySelector('.modal-backdrop')).toBeDefined()
Expand All @@ -254,6 +255,7 @@ describe('Modal', () => {

modalEl.addEventListener('shown.bs.modal', () => {
expect(modalEl.getAttribute('aria-modal')).toEqual('true')
expect(modalEl.getAttribute('role')).toEqual('dialog')
expect(modalEl.getAttribute('aria-hidden')).toEqual(null)
expect(modalEl.style.display).toEqual('block')
expect(document.querySelector('.modal-backdrop')).toBeNull()
Expand Down Expand Up @@ -731,6 +733,7 @@ describe('Modal', () => {

modalEl.addEventListener('hidden.bs.modal', () => {
expect(modalEl.getAttribute('aria-modal')).toEqual(null)
expect(modalEl.getAttribute('role')).toEqual(null)
expect(modalEl.getAttribute('aria-hidden')).toEqual('true')
expect(modalEl.style.display).toEqual('none')
expect(document.querySelector('.modal-backdrop')).toBeNull()
Expand All @@ -752,6 +755,7 @@ describe('Modal', () => {

modalEl.addEventListener('hidden.bs.modal', () => {
expect(modalEl.getAttribute('aria-modal')).toEqual(null)
expect(modalEl.getAttribute('role')).toEqual(null)
expect(modalEl.getAttribute('aria-hidden')).toEqual('true')
expect(modalEl.style.display).toEqual('none')
expect(document.querySelector('.modal-backdrop')).toBeNull()
Expand Down Expand Up @@ -859,6 +863,7 @@ describe('Modal', () => {

modalEl.addEventListener('shown.bs.modal', () => {
expect(modalEl.getAttribute('aria-modal')).toEqual('true')
expect(modalEl.getAttribute('role')).toEqual('dialog')
expect(modalEl.getAttribute('aria-hidden')).toEqual(null)
expect(modalEl.style.display).toEqual('block')
expect(document.querySelector('.modal-backdrop')).toBeDefined()
Expand Down Expand Up @@ -901,6 +906,7 @@ describe('Modal', () => {

modalEl.addEventListener('shown.bs.modal', () => {
expect(modalEl.getAttribute('aria-modal')).toEqual('true')
expect(modalEl.getAttribute('role')).toEqual('dialog')
expect(modalEl.getAttribute('aria-hidden')).toEqual(null)
expect(modalEl.style.display).toEqual('block')
expect(document.querySelector('.modal-backdrop')).toBeDefined()
Expand Down
8 changes: 4 additions & 4 deletions js/tests/visual/modal.html
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<div class="container mt-3">
<h1>Modal <small>Bootstrap Visual Test</small></h1>

<div class="modal fade" id="myModal" tabindex="-1" role="dialog" aria-labelledby="myModalLabel" aria-hidden="true">
<div class="modal fade" id="myModal" tabindex="-1" aria-labelledby="myModalLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -121,7 +121,7 @@ <h4>Overflowing text to show scroll behavior</h4>
</div>
</div>

<div class="modal fade" id="firefoxModal" tabindex="-1" role="dialog" aria-labelledby="firefoxModalLabel" aria-hidden="true">
<div class="modal fade" id="firefoxModal" tabindex="-1" aria-labelledby="firefoxModalLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -147,7 +147,7 @@ <h4 class="modal-title" id="firefoxModalLabel">Firefox Bug Test</h4>
</div>
</div>

<div class="modal fade" id="slowModal" tabindex="-1" role="dialog" aria-labelledby="slowModalLabel" aria-hidden="true" style="transition-duration: 5s;">
<div class="modal fade" id="slowModal" tabindex="-1" aria-labelledby="slowModalLabel" aria-hidden="true" style="transition-duration: 5s;">
<div class="modal-dialog" style="transition-duration: inherit;">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -194,7 +194,7 @@ <h4 class="modal-title" id="slowModalLabel">Lorem slowly</h4>
Tall body content to force the page to have a scrollbar.
</div>

<button type="button" class="btn btn-secondary btn-lg" data-toggle="modal" data-target="&#x3C;div class=&#x22;modal fade the-bad&#x22; tabindex=&#x22;-1&#x22; role=&#x22;dialog&#x22;&#x3E;&#x3C;div class=&#x22;modal-dialog&#x22; role=&#x22;document&#x22;&#x3E;&#x3C;div class=&#x22;modal-content&#x22;&#x3E;&#x3C;div class=&#x22;modal-header&#x22;&#x3E;&#x3C;button type=&#x22;button&#x22; class=&#x22;close&#x22; data-dismiss=&#x22;modal&#x22; aria-label=&#x22;Close&#x22;&#x3E;&#x3C;span aria-hidden=&#x22;true&#x22;&#x3E;&#x26;times;&#x3C;/span&#x3E;&#x3C;/button&#x3E;&#x3C;h4 class=&#x22;modal-title&#x22;&#x3E;The Bad Modal&#x3C;/h4&#x3E;&#x3C;/div&#x3E;&#x3C;div class=&#x22;modal-body&#x22;&#x3E;This modal&#x27;s HTTML source code is declared inline, inside the data-target attribute of it&#x27;s show-button&#x3C;/div&#x3E;&#x3C;/div&#x3E;&#x3C;/div&#x3E;&#x3C;/div&#x3E;">
<button type="button" class="btn btn-secondary btn-lg" data-toggle="modal" data-target="&#x3C;div class=&#x22;modal fade the-bad&#x22; tabindex=&#x22;-1&#x22;&#x3E;&#x3C;div class=&#x22;modal-dialog&#x22;&#x3E;&#x3C;div class=&#x22;modal-content&#x22;&#x3E;&#x3C;div class=&#x22;modal-header&#x22;&#x3E;&#x3C;button type=&#x22;button&#x22; class=&#x22;close&#x22; data-dismiss=&#x22;modal&#x22; aria-label=&#x22;Close&#x22;&#x3E;&#x3C;span aria-hidden=&#x22;true&#x22;&#x3E;&#x26;times;&#x3C;/span&#x3E;&#x3C;/button&#x3E;&#x3C;h4 class=&#x22;modal-title&#x22;&#x3E;The Bad Modal&#x3C;/h4&#x3E;&#x3C;/div&#x3E;&#x3C;div class=&#x22;modal-body&#x22;&#x3E;This modal&#x27;s HTTML source code is declared inline, inside the data-target attribute of it&#x27;s show-button&#x3C;/div&#x3E;&#x3C;/div&#x3E;&#x3C;/div&#x3E;&#x3C;/div&#x3E;">
Modal with an XSS inside the data-target
</button>

Expand Down
46 changes: 23 additions & 23 deletions site/content/docs/5.0/components/modal.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Keep reading for demos and usage guidelines.
Below is a _static_ modal example (meaning its `position` and `display` have been overridden). Included are the modal header, modal body (required for `padding`), and modal footer (optional). We ask that you include modal headers with dismiss actions whenever possible, or provide another explicit dismiss action.

<div class="bd-example bd-example-modal">
<div class="modal" tabindex="-1" role="dialog">
<div class="modal" tabindex="-1">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -61,7 +61,7 @@ Below is a _static_ modal example (meaning its `position` and `display` have bee
</div>

{{< highlight html >}}
<div class="modal" tabindex="-1" role="dialog">
<div class="modal" tabindex="-1">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -86,7 +86,7 @@ Below is a _static_ modal example (meaning its `position` and `display` have bee

Toggle a working modal demo by clicking the button below. It will slide down and fade in from the top of the page.

<div class="modal fade" id="exampleModalLive" tabindex="-1" role="dialog" aria-labelledby="exampleModalLiveLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalLive" tabindex="-1" aria-labelledby="exampleModalLiveLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -119,7 +119,7 @@ Toggle a working modal demo by clicking the button below. It will slide down and
</button>

<!-- Modal -->
<div class="modal fade" id="exampleModal" tabindex="-1" role="dialog" aria-labelledby="exampleModalLabel" aria-hidden="true">
<div class="modal fade" id="exampleModal" tabindex="-1" aria-labelledby="exampleModalLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -144,7 +144,7 @@ Toggle a working modal demo by clicking the button below. It will slide down and

When backdrop is set to static, the modal will not close when clicking outside it. Click the button below to try it.

<div class="modal fade" id="staticBackdropLive" data-backdrop="static" data-keyboard="false" tabindex="-1" role="dialog" aria-labelledby="staticBackdropLiveLabel" aria-hidden="true">
<div class="modal fade" id="staticBackdropLive" data-backdrop="static" data-keyboard="false" tabindex="-1" aria-labelledby="staticBackdropLiveLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -177,7 +177,7 @@ When backdrop is set to static, the modal will not close when clicking outside i
</button>

<!-- Modal -->
<div class="modal fade" id="staticBackdrop" data-backdrop="static" data-keyboard="false" tabindex="-1" role="dialog" aria-labelledby="staticBackdropLabel" aria-hidden="true">
<div class="modal fade" id="staticBackdrop" data-backdrop="static" data-keyboard="false" tabindex="-1" aria-labelledby="staticBackdropLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -203,7 +203,7 @@ When backdrop is set to static, the modal will not close when clicking outside i

When modals become too long for the user's viewport or device, they scroll independent of the page itself. Try the demo below to see what we mean.

<div class="modal fade" id="exampleModalLong" tabindex="-1" role="dialog" aria-labelledby="exampleModalLongTitle" aria-hidden="true">
<div class="modal fade" id="exampleModalLong" tabindex="-1" aria-labelledby="exampleModalLongTitle" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -248,7 +248,7 @@ When modals become too long for the user's viewport or device, they scroll indep

You can also create a scrollable modal that allows scroll the modal body by adding `.modal-dialog-scrollable` to `.modal-dialog`.

<div class="modal fade" id="exampleModalScrollable" tabindex="-1" role="dialog" aria-labelledby="exampleModalScrollableTitle" aria-hidden="true">
<div class="modal fade" id="exampleModalScrollable" tabindex="-1" aria-labelledby="exampleModalScrollableTitle" aria-hidden="true">
<div class="modal-dialog modal-dialog-scrollable">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -302,7 +302,7 @@ You can also create a scrollable modal that allows scroll the modal body by addi

Add `.modal-dialog-centered` to `.modal-dialog` to vertically center the modal.

<div class="modal fade" id="exampleModalCenter" tabindex="-1" role="dialog" aria-labelledby="exampleModalCenterTitle" aria-hidden="true">
<div class="modal fade" id="exampleModalCenter" tabindex="-1" aria-labelledby="exampleModalCenterTitle" aria-hidden="true">
<div class="modal-dialog modal-dialog-centered">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -322,7 +322,7 @@ Add `.modal-dialog-centered` to `.modal-dialog` to vertically center the modal.
</div>
</div>

<div class="modal fade" id="exampleModalCenteredScrollable" tabindex="-1" role="dialog" aria-labelledby="exampleModalCenteredScrollableTitle" aria-hidden="true">
<div class="modal fade" id="exampleModalCenteredScrollable" tabindex="-1" aria-labelledby="exampleModalCenteredScrollableTitle" aria-hidden="true">
<div class="modal-dialog modal-dialog-centered modal-dialog-scrollable">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -371,7 +371,7 @@ Add `.modal-dialog-centered` to `.modal-dialog` to vertically center the modal.

[Tooltips]({{< docsref "/components/tooltips" >}}) and [popovers]({{< docsref "/components/popovers" >}}) can be placed within modals as needed. When modals are closed, any tooltips and popovers within are also automatically dismissed.

<div class="modal fade" id="exampleModalPopovers" tabindex="-1" role="dialog" aria-labelledby="exampleModalPopoversLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalPopovers" tabindex="-1" aria-labelledby="exampleModalPopoversLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -415,7 +415,7 @@ Add `.modal-dialog-centered` to `.modal-dialog` to vertically center the modal.

Utilize the Bootstrap grid system within a modal by nesting `.container-fluid` within the `.modal-body`. Then, use the normal grid system classes as you would anywhere else.

<div class="modal fade" id="gridSystemModal" tabindex="-1" role="dialog" aria-labelledby="gridModalLabel" aria-hidden="true">
<div class="modal fade" id="gridSystemModal" tabindex="-1" aria-labelledby="gridModalLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -506,7 +506,7 @@ Below is a live demo followed by example HTML and JavaScript. For more informati
<button type="button" class="btn btn-primary" data-toggle="modal" data-target="#exampleModal" data-whatever="@fat">Open modal for @fat</button>
<button type="button" class="btn btn-primary" data-toggle="modal" data-target="#exampleModal" data-whatever="@getbootstrap">Open modal for @getbootstrap</button>

<div class="modal fade" id="exampleModal" tabindex="-1" role="dialog" aria-labelledby="exampleModalLabel" aria-hidden="true">
<div class="modal fade" id="exampleModal" tabindex="-1" aria-labelledby="exampleModalLabel" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -566,7 +566,7 @@ If you want for example a zoom-in animation, you can set `$modal-fade-transform:
For modals that simply appear rather than fade in to view, remove the `.fade` class from your modal markup.

{{< highlight html >}}
<div class="modal" tabindex="-1" role="dialog" aria-labelledby="..." aria-hidden="true">
<div class="modal" tabindex="-1" aria-labelledby="..." aria-hidden="true">
...
</div>
{{< /highlight >}}
Expand All @@ -577,7 +577,7 @@ If the height of a modal changes while it is open, you should call `myModal.hand

### Accessibility

Be sure to add `role="dialog"` and `aria-labelledby="..."`, referencing the modal title, to `.modal`. Additionally, you may give a description of your modal dialog with `aria-describedby` on `.modal`.
Be sure to add `aria-labelledby="..."`, referencing the modal title, to `.modal`. Additionally, you may give a description of your modal dialog with `aria-describedby` on `.modal`.
rohit2sharma95 marked this conversation as resolved.
Show resolved Hide resolved

### Embedding YouTube videos

Expand Down Expand Up @@ -633,7 +633,7 @@ Our default modal without modifier class constitutes the "medium" size modal.
<div class="modal-dialog modal-sm">...</div>
{{< /highlight >}}

<div class="modal fade" id="exampleModalXl" tabindex="-1" role="dialog" aria-labelledby="exampleModalXlLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalXl" tabindex="-1" aria-labelledby="exampleModalXlLabel" aria-hidden="true">
<div class="modal-dialog modal-xl">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -649,7 +649,7 @@ Our default modal without modifier class constitutes the "medium" size modal.
</div>
</div>

<div class="modal fade" id="exampleModalLg" tabindex="-1" role="dialog" aria-labelledby="exampleModalLgLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalLg" tabindex="-1" aria-labelledby="exampleModalLgLabel" aria-hidden="true">
<div class="modal-dialog modal-lg">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -665,7 +665,7 @@ Our default modal without modifier class constitutes the "medium" size modal.
</div>
</div>

<div class="modal fade" id="exampleModalSm" tabindex="-1" role="dialog" aria-labelledby="exampleModalSmLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalSm" tabindex="-1" aria-labelledby="exampleModalSmLabel" aria-hidden="true">
<div class="modal-dialog modal-sm">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -731,7 +731,7 @@ Another override is the option to pop up a modal that covers the user viewport,
</div>
{{< /highlight >}}

<div class="modal fade" id="exampleModalFullscreen" tabindex="-1" role="dialog" aria-labelledby="exampleModalFullscreenLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalFullscreen" tabindex="-1" aria-labelledby="exampleModalFullscreenLabel" aria-hidden="true">
<div class="modal-dialog modal-fullscreen">
<div class="modal-content">
<div class="modal-header">
Expand Down Expand Up @@ -767,7 +767,7 @@ Another override is the option to pop up a modal that covers the user viewport,
</div>
</div>

<div class="modal fade" id="exampleModalFullscreenSm" tabindex="-1" role="dialog" aria-labelledby="exampleModalFullscreenSmLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalFullscreenSm" tabindex="-1" aria-labelledby="exampleModalFullscreenSmLabel" aria-hidden="true">
<div class="modal-dialog modal-fullscreen-sm-down">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -786,7 +786,7 @@ Another override is the option to pop up a modal that covers the user viewport,
</div>
</div>

<div class="modal fade" id="exampleModalFullscreenMd" tabindex="-1" role="dialog" aria-labelledby="exampleModalFullscreenMdLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalFullscreenMd" tabindex="-1" aria-labelledby="exampleModalFullscreenMdLabel" aria-hidden="true">
<div class="modal-dialog modal-fullscreen-md-down">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -805,7 +805,7 @@ Another override is the option to pop up a modal that covers the user viewport,
</div>
</div>

<div class="modal fade" id="exampleModalFullscreenLg" tabindex="-1" role="dialog" aria-labelledby="exampleModalFullscreenLgLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalFullscreenLg" tabindex="-1" aria-labelledby="exampleModalFullscreenLgLabel" aria-hidden="true">
<div class="modal-dialog modal-fullscreen-lg-down">
<div class="modal-content">
<div class="modal-header">
Expand All @@ -824,7 +824,7 @@ Another override is the option to pop up a modal that covers the user viewport,
</div>
</div>

<div class="modal fade" id="exampleModalFullscreenXl" tabindex="-1" role="dialog" aria-labelledby="exampleModalFullscreenXlLabel" aria-hidden="true">
<div class="modal fade" id="exampleModalFullscreenXl" tabindex="-1" aria-labelledby="exampleModalFullscreenXlLabel" aria-hidden="true">
<div class="modal-dialog modal-fullscreen-xl-down">
<div class="modal-content">
<div class="modal-header">
Expand Down