Changeset 143299 in webkit


Ignore:
Timestamp:
Feb 18, 2013 11:22:32 PM (11 years ago)
Author:
aestes@apple.com
Message:

Focusing a new frame (via window.focus()) should blur the active element in the current frame
https://bugs.webkit.org/show_bug.cgi?id=110172

Reviewed by Ryosuke Niwa.

Source/WebCore:

When a change in the focused node crosses a frame boundary, WebKit
doesn't always succeed in blurring the old focused node before focusing
the new one.

Each document remembers its focused node, and a Page-scoped
FocusController remembers the focused frame. If a new focused node is
in a different frame than the focused frame, FocusController tells the
old frame's document to clear its focused node before focusing the new
one (and remembering the new frame).

Unfortunately, web content can confuse FocusController by calling
window.focus() at the wrong time. Since window.focus() changes
FocusController's focused frame without focusing a new node,
FocusController won't think that a frame boundary is being crossed if a
node in this frame is later focused. Therefore it won't clear the old
frame's focused node (it won't even know which frame contained the old
focused node), causing at least two bugs:

1) The node in the old frame will not receive a blur event.
2) Calling document.activeElement on the main frame will return the

previously focused node, but the HTML5 spec says it should return
the frame owner element if a subframe has focus.

Fix both of these bugs by explicitly clearing the current frame's
focused node if window.focus() changes the focused frame. This fix
carries some compatibility risk by changing a long-standing behavior
of the engine (we've had this bug since the beginning of the project,
AFAICT). On the upside, it matches the behavior of both Firefox and IE,
matches what HTML5 says about subframe focus, and fixes at least one
well-known enterprise web app.

Tests: fast/dom/HTMLDocument/active-element-frames.html

fast/frames/frame-focus-blurs-active-element.html

  • page/DOMWindow.cpp:

(WebCore::DOMWindow::focus): If the frame being focused is not the same
as the currently focused frame, clear the currently focused frame's
focused node.

LayoutTests:

  • fast/dom/HTMLDocument/active-element-frames-expected.txt:
  • fast/dom/HTMLDocument/active-element-frames.html: Modified to run the

test a second time, focusing each element's frame before focusing the
element itself.

  • fast/frames/frame-focus-blurs-active-element-expected.txt: Added.
  • fast/frames/frame-focus-blurs-active-element.html: Added a test that

verifies a blur event is fired on the active element when a new frame
is focused.

Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r143296 r143299  
     12013-02-18  Andy Estes  <aestes@apple.com>
     2
     3        Focusing a new frame (via window.focus()) should blur the active element in the current frame
     4        https://bugs.webkit.org/show_bug.cgi?id=110172
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        * fast/dom/HTMLDocument/active-element-frames-expected.txt:
     9        * fast/dom/HTMLDocument/active-element-frames.html: Modified to run the
     10        test a second time, focusing each element's frame before focusing the
     11        element itself.
     12        * fast/frames/frame-focus-blurs-active-element-expected.txt: Added.
     13        * fast/frames/frame-focus-blurs-active-element.html: Added a test that
     14        verifies a blur event is fired on the active element when a new frame
     15        is focused.
     16
    1172013-02-18  Kondapally Kalyan  <kalyan.kondapally@intel.com>
    218
  • trunk/LayoutTests/fast/dom/HTMLDocument/active-element-frames-expected.txt

    r86707 r143299  
    22
    33 
     4
    45Focusing top/input-0
    56PASS: top document.activeElement is top/input-0
     
    3738PASS: top/iframe-2/iframe-4 document.activeElement is top/iframe-2/iframe-4/input-4
    3839
     40Focusing top/input-0 after focusing its window
     41PASS: top document.activeElement is top/input-0
     42PASS: top/iframe-1 document.activeElement is top/iframe-1/body-1
     43PASS: top/iframe-2 document.activeElement is top/iframe-2/body-2
     44PASS: top/iframe-1/iframe-3 document.activeElement is top/iframe-1/iframe-3/body-3
     45PASS: top/iframe-2/iframe-4 document.activeElement is top/iframe-2/iframe-4/body-4
     46
     47Focusing top/iframe-1/input-1 after focusing its window
     48PASS: top document.activeElement is top/iframe-1
     49PASS: top/iframe-1 document.activeElement is top/iframe-1/input-1
     50PASS: top/iframe-2 document.activeElement is top/iframe-2/body-2
     51PASS: top/iframe-1/iframe-3 document.activeElement is top/iframe-1/iframe-3/body-3
     52PASS: top/iframe-2/iframe-4 document.activeElement is top/iframe-2/iframe-4/body-4
     53
     54Focusing top/iframe-2/input-2 after focusing its window
     55PASS: top document.activeElement is top/iframe-2
     56PASS: top/iframe-1 document.activeElement is top/iframe-1/body-1
     57PASS: top/iframe-2 document.activeElement is top/iframe-2/input-2
     58PASS: top/iframe-1/iframe-3 document.activeElement is top/iframe-1/iframe-3/body-3
     59PASS: top/iframe-2/iframe-4 document.activeElement is top/iframe-2/iframe-4/body-4
     60
     61Focusing top/iframe-1/iframe-3/input-3 after focusing its window
     62PASS: top document.activeElement is top/iframe-1
     63PASS: top/iframe-1 document.activeElement is top/iframe-1/iframe-3
     64PASS: top/iframe-2 document.activeElement is top/iframe-2/body-2
     65PASS: top/iframe-1/iframe-3 document.activeElement is top/iframe-1/iframe-3/input-3
     66PASS: top/iframe-2/iframe-4 document.activeElement is top/iframe-2/iframe-4/body-4
     67
     68Focusing top/iframe-2/iframe-4/input-4 after focusing its window
     69PASS: top document.activeElement is top/iframe-2
     70PASS: top/iframe-1 document.activeElement is top/iframe-1/body-1
     71PASS: top/iframe-2 document.activeElement is top/iframe-2/iframe-4
     72PASS: top/iframe-1/iframe-3 document.activeElement is top/iframe-1/iframe-3/body-3
     73PASS: top/iframe-2/iframe-4 document.activeElement is top/iframe-2/iframe-4/input-4
     74
  • trunk/LayoutTests/fast/dom/HTMLDocument/active-element-frames.html

    r120792 r143299  
    5757}
    5858
     59function setFocus(node, shouldFocusWindow)
     60{
     61    print('\nFocusing ' + describe(node) + (shouldFocusWindow ? ' after focusing its window' : ''));
     62    if (shouldFocusWindow)
     63        node.ownerDocument.defaultView.focus();
     64    node.focus();
     65}
     66
     67function test(shouldFocusWindow)
     68{
     69    setFocus(input0, shouldFocusWindow);
     70    assertActiveElement(doc0, input0);
     71    assertActiveElement(doc1, doc1.body);
     72    assertActiveElement(doc2, doc2.body);
     73    assertActiveElement(doc3, doc3.body);
     74    assertActiveElement(doc4, doc4.body);
     75
     76    setFocus(input1, shouldFocusWindow);
     77    assertActiveElement(doc0, iframe1);
     78    assertActiveElement(doc1, input1);
     79    assertActiveElement(doc2, doc2.body);
     80    assertActiveElement(doc3, doc3.body);
     81    assertActiveElement(doc4, doc4.body);
     82
     83    setFocus(input2, shouldFocusWindow);
     84    assertActiveElement(doc0, iframe2);
     85    assertActiveElement(doc1, doc1.body);
     86    assertActiveElement(doc2, input2);
     87    assertActiveElement(doc3, doc3.body);
     88    assertActiveElement(doc4, doc4.body);
     89
     90    setFocus(input3, shouldFocusWindow);
     91    assertActiveElement(doc0, iframe1);
     92    assertActiveElement(doc1, iframe3);
     93    assertActiveElement(doc2, doc2.body);
     94    assertActiveElement(doc3, input3);
     95    assertActiveElement(doc4, doc4.body);
     96
     97    setFocus(input4, shouldFocusWindow);
     98    assertActiveElement(doc0, iframe2);
     99    assertActiveElement(doc1, doc1.body);
     100    assertActiveElement(doc2, iframe4);
     101    assertActiveElement(doc3, doc3.body);
     102    assertActiveElement(doc4, input4);
     103}
     104
    59105if (window.testRunner)
    60106    testRunner.dumpAsText();
    61107
    62 print('Focusing ' + describe(input0));
    63 input0.focus();
    64 assertActiveElement(doc0, input0);
    65 assertActiveElement(doc1, doc1.body);
    66 assertActiveElement(doc2, doc2.body);
    67 assertActiveElement(doc3, doc3.body);
    68 assertActiveElement(doc4, doc4.body);
     108// Test focusing elements directly.
     109test(false);
    69110
    70 print('\nFocusing ' + describe(input1));
    71 input1.focus();
    72 assertActiveElement(doc0, iframe1);
    73 assertActiveElement(doc1, input1);
    74 assertActiveElement(doc2, doc2.body);
    75 assertActiveElement(doc3, doc3.body);
    76 assertActiveElement(doc4, doc4.body);
    77 
    78 print('\nFocusing ' + describe(input2));
    79 input2.focus();
    80 assertActiveElement(doc0, iframe2);
    81 assertActiveElement(doc1, doc1.body);
    82 assertActiveElement(doc2, input2);
    83 assertActiveElement(doc3, doc3.body);
    84 assertActiveElement(doc4, doc4.body);
    85 
    86 print('\nFocusing ' + describe(input3));
    87 input3.focus();
    88 assertActiveElement(doc0, iframe1);
    89 assertActiveElement(doc1, iframe3);
    90 assertActiveElement(doc2, doc2.body);
    91 assertActiveElement(doc3, input3);
    92 assertActiveElement(doc4, doc4.body);
    93 
    94 print('\nFocusing ' + describe(input4));
    95 input4.focus();
    96 assertActiveElement(doc0, iframe2);
    97 assertActiveElement(doc1, doc1.body);
    98 assertActiveElement(doc2, iframe4);
    99 assertActiveElement(doc3, doc3.body);
    100 assertActiveElement(doc4, input4);
     111// Test focusing elements after focusing their windows first.
     112test(true);
    101113
    102114</script>
  • trunk/Source/WebCore/ChangeLog

    r143295 r143299  
     12013-02-18  Andy Estes  <aestes@apple.com>
     2
     3        Focusing a new frame (via window.focus()) should blur the active element in the current frame
     4        https://bugs.webkit.org/show_bug.cgi?id=110172
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        When a change in the focused node crosses a frame boundary, WebKit
     9        doesn't always succeed in blurring the old focused node before focusing
     10        the new one.
     11
     12        Each document remembers its focused node, and a Page-scoped
     13        FocusController remembers the focused frame. If a new focused node is
     14        in a different frame than the focused frame, FocusController tells the
     15        old frame's document to clear its focused node before focusing the new
     16        one (and remembering the new frame).
     17
     18        Unfortunately, web content can confuse FocusController by calling
     19        window.focus() at the wrong time. Since window.focus() changes
     20        FocusController's focused frame without focusing a new node,
     21        FocusController won't think that a frame boundary is being crossed if a
     22        node in this frame is later focused. Therefore it won't clear the old
     23        frame's focused node (it won't even know which frame contained the old
     24        focused node), causing at least two bugs:
     25
     26        1) The node in the old frame will not receive a blur event.
     27        2) Calling document.activeElement on the main frame will return the
     28           previously focused node, but the HTML5 spec says it should return
     29           the frame owner element if a subframe has focus.
     30
     31        Fix both of these bugs by explicitly clearing the current frame's
     32        focused node if window.focus() changes the focused frame. This fix
     33        carries some compatibility risk by changing a long-standing behavior
     34        of the engine (we've had this bug since the beginning of the project,
     35        AFAICT). On the upside, it matches the behavior of both Firefox and IE,
     36        matches what HTML5 says about subframe focus, and fixes at least one
     37        well-known enterprise web app.
     38
     39        Tests: fast/dom/HTMLDocument/active-element-frames.html
     40               fast/frames/frame-focus-blurs-active-element.html
     41
     42        * page/DOMWindow.cpp:
     43        (WebCore::DOMWindow::focus): If the frame being focused is not the same
     44        as the currently focused frame, clear the currently focused frame's
     45        focused node.
     46
    1472013-02-18  Simon Fraser  <simon.fraser@apple.com>
    248
  • trunk/Source/WebCore/page/DOMWindow.cpp

    r143295 r143299  
    5959#include "ExceptionCodePlaceholder.h"
    6060#include "FloatRect.h"
     61#include "FocusController.h"
    6162#include "Frame.h"
    6263#include "FrameLoadRequest.h"
     
    933934        return;
    934935
     936    // Clear the current frame's focused node if a new frame is about to be focused.
     937    Frame* focusedFrame = page->focusController()->focusedFrame();
     938    if (focusedFrame && focusedFrame != m_frame)
     939        focusedFrame->document()->setFocusedNode(0);
     940
    935941    m_frame->eventHandler()->focusDocumentView();
    936942}
Note: See TracChangeset for help on using the changeset viewer.