Skip to content

Find active element within iframe#8290

Merged
msft-github-bot merged 5 commits into
microsoft:masterfrom
aftab-hassan:active-element-in-iframe-fix
Mar 15, 2019
Merged

Find active element within iframe#8290
msft-github-bot merged 5 commits into
microsoft:masterfrom
aftab-hassan:active-element-in-iframe-fix

Conversation

@aftab-hassan

@aftab-hassan aftab-hassan commented Mar 13, 2019

Copy link
Copy Markdown
Contributor

Pull request checklist

Description of changes

Creating this PR and closing 8071 due to a bad merge. The comments on the linked PR are resolved

  1. Types were added
  2. In order to avoid the possibility of an iFrame referring to itself, resulting in an infinite loop, the following check was added
if (tempActive === tempActiveLastSeen) {
   break;
}
  1. Could not add a unit test for this one, since this has been a difficult scenario to unit test.

Broader context

In Popup.tsx, it tries to determine the active element of the site by doing

this._originalFocusedElement = getDocument()!.activeElement as HTMLElement;
The issue with that is if there is a frame, the active element will be a reference to the head of the frame, instead of the elements within it.

The fix is to find the active element using getDocument()!.contentWindow.document.activeElement in case of an iframe being present, as is the case mentioned in the bug.

The above PR addresses the conditions

  1. The iFrame contains an active element, as is the case reported in the issue
  2. The iFrame contains a multi level nesting of iFrames, which contains an active element. To address this, the while loop was added
while (tempActive !== null && tempActive instanceof HTMLIFrameElement) {
  if (tempActive.contentDocument !== null && tempActive.contentDocument.activeElement !== null) {
    tempActiveLastSeen = tempActive;
    tempActive = tempActive.contentDocument.activeElement;
    if (tempActive === tempActiveLastSeen) {
      break;
    }
  }
}

Focus areas to test

Create a page with a panel containing an iframe and a focus element like a textbox.
On hitting the escape on the panel, open up a dialog box
On hitting escape on the dialog, control goes back to the focus element (textbox)

Microsoft Reviewers: Open in CodeFlow

@aftab-hassan

Copy link
Copy Markdown
Contributor Author

A simpler version of this fix would be to just do an if-conditional

if (tempActive !== null && tempActive instanceof HTMLIFrameElement) {
      if (tempActive.contentDocument !== null && tempActive.contentDocument.activeElement !== null) {
        tempActive = tempActive.contentDocument.activeElement;
      }
    }

This will fix the issue reported. However, it would not support the case where the iFrame contains an iFrame which contains an active element (the nesting case mentioned in 2 above). It is probably not a common case to have nested iFrames, but useful to support.

@size-auditor

size-auditor Bot commented Mar 13, 2019

Copy link
Copy Markdown
Bundle test Size (minified) Diff from master
HoverCard 94.229 kB ExceedsBaseline     205 bytes
MessageBar 166.155 kB ExceedsBaseline     205 bytes
ComboBox 213.861 kB ExceedsBaseline     205 bytes
CommandBar 176.161 kB ExceedsBaseline     205 bytes
ContextualMenu 140.61 kB ExceedsBaseline     205 bytes
DatePicker 195.072 kB ExceedsBaseline     205 bytes
Grid 159.534 kB ExceedsBaseline     205 bytes
Popup 23.939 kB ExceedsBaseline     205 bytes
Pivot 164.397 kB ExceedsBaseline     205 bytes
Pickers 248.769 kB ExceedsBaseline     205 bytes
Dialog 175.349 kB ExceedsBaseline     205 bytes
PersonaCoin 108.845 kB ExceedsBaseline     205 bytes
Persona 108.845 kB ExceedsBaseline     205 bytes
Panel 173.867 kB ExceedsBaseline     205 bytes
Dropdown 201.231 kB ExceedsBaseline     205 bytes
DocumentCard 188.175 kB ExceedsBaseline     205 bytes
Nav 165.888 kB ExceedsBaseline     205 bytes
SearchBox 163.166 kB ExceedsBaseline     205 bytes
Modal 74.586 kB ExceedsBaseline     205 bytes
SelectedItemsList 200.462 kB ExceedsBaseline     205 bytes
Keytip 82.314 kB ExceedsBaseline     205 bytes
Breadcrumb 174.7 kB ExceedsBaseline     205 bytes
Button 168.524 kB ExceedsBaseline     205 bytes
TeachingBubble 170.916 kB ExceedsBaseline     205 bytes
SwatchColorPicker 172.423 kB ExceedsBaseline     205 bytes
Callout 85.141 kB ExceedsBaseline     205 bytes
FloatingPicker 210.891 kB ExceedsBaseline     205 bytes
SpinButton 169.477 kB ExceedsBaseline     205 bytes
KeytipLayer 98.44 kB ExceedsBaseline     205 bytes
Facepile 184.578 kB ExceedsBaseline     205 bytes
Tooltip 84.99 kB ExceedsBaseline     205 bytes

ExceedsTolerance  Exceeds Tolerance     ExceedsBaseline  Exceeds Baseline     BelowBaseline  Below Baseline     1 kB = 1000 bytes

@KevinTCoughlin KevinTCoughlin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit: added some minor comments about adding comments :D

Comment thread packages/office-ui-fabric-react/src/components/Popup/Popup.tsx
@msft-github-bot

Copy link
Copy Markdown
Contributor

Hello @KevinTCoughlin!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

@msft-github-bot
msft-github-bot merged commit 130b998 into microsoft:master Mar 15, 2019
@msft-github-bot

Copy link
Copy Markdown
Contributor

🎉office-ui-fabric-react@v6.157.0 has been released which incorporates this pull request.:tada:

Handy links:

@aftab-hassan
aftab-hassan deleted the active-element-in-iframe-fix branch June 15, 2019 02:14
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Set Focus correctly in iframe (active element issue)

4 participants