Add a simple implementation of dialog.showModal() so it can later be used to test the top layer (not yet implemented, bug 84796). Modal dialogs should be in the top layer.
Created attachment 165312 [details] Patch
Comment on attachment 165312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165312&action=review > Source/WebCore/html/HTMLDialogElement.cpp:70 > + if (fastHasAttribute(openAttr) || !inDocument()) { > + ec = INVALID_STATE_ERR; > + return; > + } What should happen if the document is not attached to any pages? We can make such documents by DOMImplementation::createHTMLDocument(), etc. > LayoutTests/fast/dom/HTMLDialogElement/dialog-show-modal.html:17 > +shouldBe("computedStyle.getPropertyValue('display')", "'none'"); We had better use shouldBeEqualToString() to avoid nested quotation marks. > LayoutTests/fast/dom/HTMLDialogElement/dialog-show-modal.html:21 > +dialog.showModal(); > +computedStyle = window.getComputedStyle(dialog, null); > +shouldBe("computedStyle.getPropertyValue('display')", "'block'"); Will this work when we implement modal behavior for showModal()?
Comment on attachment 165312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165312&action=review >> Source/WebCore/html/HTMLDialogElement.cpp:70 >> + } > > What should happen if the document is not attached to any pages? We can make such documents by DOMImplementation::createHTMLDocument(), etc. It looks like the spec doesn't say anything about this case. I think showModal() can execute as normal. When there is no document, it can't proceed because there is no "pending dialog stack". >> LayoutTests/fast/dom/HTMLDialogElement/dialog-show-modal.html:21 >> +shouldBe("computedStyle.getPropertyValue('display')", "'block'"); > > Will this work when we implement modal behavior for showModal()? Yes, I believe modal doesn't block JavaScript execution. It just blocks user interaction with the rest of the document.
Created attachment 165317 [details] Patch
Comment on attachment 165317 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165317&action=review Almost ok. Some nits. > LayoutTests/fast/dom/HTMLDialogElement/dialog-show-modal-expected.txt:12 > +PASS computedStyle.getPropertyValue('display') is "none" > +PASS computedStyle.getPropertyValue('display') is "block" > +PASS dialog.showModal(); threw exception Error: INVALID_STATE_ERR: DOM Exception 11. > +PASS computedStyle.getPropertyValue('display') is "none" > +PASS dialog.showModal(); threw exception Error: INVALID_STATE_ERR: DOM Exception 11. > +PASS dialog.open is false > +PASS dialog.open is true The readability of test result is not good. Please print some explanations about what we are testing. > LayoutTests/fast/dom/HTMLDialogElement/dialog-show-modal.html:23 > +// "If dialog already has an open attribute, then throw an InvalidStateError exception." This sentence should be printed with debug(). > LayoutTests/fast/dom/HTMLDialogElement/dialog-show-modal.html:30 > +// "If dialog is not in a Document, then throw an InvalidStateError exception." ditto.
Created attachment 165319 [details] Patch
Comment on attachment 165319 [details] Patch ok
Comment on attachment 165319 [details] Patch Clearing flags on attachment: 165319 Committed r129330: <http://trac.webkit.org/changeset/129330>
All reviewed patches have been landed. Closing bug.