Changeset 279950 in webkit


Ignore:
Timestamp:
Jul 15, 2021, 9:16:40 AM (4 years ago)
Author:
ntim@apple.com
Message:

<dialog> element: do not perform close() method steps when removing open attribute.
​https://bugs.webkit.org/show_bug.cgi?id=227872

Reviewed by Chris Dumez.

Test: web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-open.html

The close function now follows the steps at: ​https://html.spec.whatwg.org/multipage/interactive-elements.html#close-the-dialog

LayoutTests/imported/w3c:

  • web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-open-expected.txt:

Source/WebCore:

  • html/HTMLDialogElement.cpp:

(WebCore::HTMLDialogElement::show):
(WebCore::HTMLDialogElement::showModal):
(WebCore::HTMLDialogElement::close):
(WebCore::HTMLDialogElement::dispatchPendingEvent):
(WebCore::HTMLDialogElement::isOpen const): Deleted.
(WebCore::HTMLDialogElement::returnValue): Deleted.
(WebCore::HTMLDialogElement::setReturnValue): Deleted.
(WebCore::HTMLDialogElement::parseAttribute): Deleted.
(WebCore::HTMLDialogElement::isModal const): Deleted.

  • html/HTMLDialogElement.h:
Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r279943 r279950  
     12021-07-15  Tim Nguyen  <ntim@apple.com>
     2
     3        <dialog> element: do not perform close() method steps when removing open attribute.
     4        https://bugs.webkit.org/show_bug.cgi?id=227872
     5
     6        Reviewed by Chris Dumez.
     7
     8        Test: web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-open.html
     9
     10        The close function now follows the steps at: https://html.spec.whatwg.org/multipage/interactive-elements.html#close-the-dialog
     11
     12        * web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-open-expected.txt:
     13
    1142021-07-15  Tim Nguyen  <ntim@apple.com>
    215

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-open-expected.txt

    r279943 r279950  
    55PASS On getting, the IDL open attribute must return true if the content open attribute is set, and false if it is absent.
    66PASS On setting, the content open attribute must be removed if the IDL open attribute is set to false, and must be present if the IDL open attribute is set to true.
    7 FAIL On setting it to false, the close event should not be fired assert_unreached: close event should not be fired when just setting the open attribute Reached unreachable code
     7PASS On setting it to false, the close event should not be fired
    88

  • trunk/Source/WebCore/ChangeLog

    r279949 r279950  
     12021-07-15  Tim Nguyen  <ntim@apple.com>
     2
     3        <dialog> element: do not perform close() method steps when removing open attribute.
     4        https://bugs.webkit.org/show_bug.cgi?id=227872
     5
     6        Reviewed by Chris Dumez.
     7
     8        Test: web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-open.html
     9
     10        The close function now follows the steps at: https://html.spec.whatwg.org/multipage/interactive-elements.html#close-the-dialog
     11
     12        * html/HTMLDialogElement.cpp:
     13        (WebCore::HTMLDialogElement::show):
     14        (WebCore::HTMLDialogElement::showModal):
     15        (WebCore::HTMLDialogElement::close):
     16        (WebCore::HTMLDialogElement::dispatchPendingEvent):
     17        (WebCore::HTMLDialogElement::isOpen const): Deleted.
     18        (WebCore::HTMLDialogElement::returnValue): Deleted.
     19        (WebCore::HTMLDialogElement::setReturnValue): Deleted.
     20        (WebCore::HTMLDialogElement::parseAttribute): Deleted.
     21        (WebCore::HTMLDialogElement::isModal const): Deleted.
     22        * html/HTMLDialogElement.h:
     23
    1242021-07-15  Jer Noble  <jer.noble@apple.com>
    225

  • trunk/Source/WebCore/html/HTMLDialogElement.cpp

    r279780 r279950  
    5454}
    5555
    56 bool HTMLDialogElement::isOpen() const
    57 {
    58     return m_isOpen;
    59 }
    60 
    61 const String& HTMLDialogElement::returnValue()
    62 {
    63     return m_returnValue;
    64 }
    65 
    66 void HTMLDialogElement::setReturnValue(String&& returnValue)
    67 {
    68     m_returnValue = WTFMove(returnValue);
    69 }
    70 
    7156void HTMLDialogElement::show()
    7257{
    … …  
    7459    if (isOpen())
    7560        return;
    76    
     61
    7762    setBooleanAttribute(openAttr, true);
    7863}
    … …  
    9075    setBooleanAttribute(openAttr, true);
    9176
     77    m_isModal = true;
     78
     79    // FIXME: Only add dialog to top layer if it's not already in it. (webkit.org/b/227907)
    9280    document().addToTopLayer(*this);
    93     m_isModal = true;
     81
     82    // FIXME: Add steps 8 & 9 from spec. (webkit.org/b/227537)
    9483
    9584    return { };
    9685}
    9786
    98 void HTMLDialogElement::close(const String& returnValue)
     87void HTMLDialogElement::close(const String& result)
    9988{
    10089    if (!isOpen())
    10190        return;
    102    
     91
    10392    setBooleanAttribute(openAttr, false);
    10493
    105     if (!returnValue.isNull())
    106         m_returnValue = returnValue;
     94    m_isModal = false;
     95
     96    if (!result.isNull())
     97        m_returnValue = result;
     98
     99    // FIXME: Only remove dialog from top layer if it's inside it. (webkit.org/b/227907)
     100    document().removeFromTopLayer(*this);
     101
     102    // FIXME: Add step 6 from spec. (webkit.org/b/227537)
     103
     104    dialogCloseEventSender().cancelEvent(*this);
     105    dialogCloseEventSender().dispatchEventSoon(*this);
    107106}
    108107
    … …  
    113112}
    114113
    115 void HTMLDialogElement::parseAttribute(const QualifiedName& name, const AtomString& value)
    116 {
    117     if (name == openAttr) {
    118         bool oldValue = m_isOpen;
    119         m_isOpen = !value.isNull();
    120 
    121         // Emit close event
    122         if (oldValue != m_isOpen && !m_isOpen) {
    123             if (m_isModal) {
    124                 document().removeFromTopLayer(*this);
    125                 m_isModal = false;
    126             }
    127 
    128             dialogCloseEventSender().cancelEvent(*this);
    129             dialogCloseEventSender().dispatchEventSoon(*this);
    130         }
    131         return;
    132     }
    133    
    134     HTMLElement::parseAttribute(name, value);
    135114}
    136 
    137 bool HTMLDialogElement::isModal() const
    138 {
    139     return m_isModal;
    140 }
    141 
    142 }

  • trunk/Source/WebCore/html/HTMLDialogElement.h

    r279643 r279950  
    3838    template<typename... Args> static Ref<HTMLDialogElement> create(Args&&... args) { return adoptRef(*new HTMLDialogElement(std::forward<Args>(args)...)); }
    3939    ~HTMLDialogElement();
    40    
    41     bool isOpen() const;
    4240
    43     const String& returnValue();
    44     void setReturnValue(String&&);
     41    bool isOpen() const { return hasAttribute(HTMLNames::openAttr); }
     42
     43    const String& returnValue() const { return m_returnValue; }
     44    void setReturnValue(String&& value) { m_returnValue = WTFMove(value); }
    4545
    4646    void show();
    … …  
    5050    void dispatchPendingEvent(DialogEventSender*);
    5151
    52     bool isModal() const;
     52    bool isModal() const { return m_isModal; };
    5353
    5454private:
    5555    HTMLDialogElement(const QualifiedName&, Document&);
    5656
    57     void parseAttribute(const QualifiedName&, const AtomString&) final;
    58 
    5957    String m_returnValue;
    6058    bool m_isModal { false };
    61     bool m_isOpen { false };
    6259};
    6360
Note: See TracChangeset for help on using the changeset viewer.