Update DevTools code relying on toolbox.win.parent or toolbox.win.top

RESOLVED FIXED in Firefox 68

Status

enhancement
P2
normal
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: jdescottes, Assigned: jdescottes)

Tracking

(Blocks 1 bug)

unspecified
Firefox 68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(5 attachments)

Assignee

Description

2 months ago

In Bug 1539979 we want to use a frame with type=content to load DevTools. After that change we can no longer use win.top or win.parent to reach the Chrome window parent of the toolbox window. Instead we should use the topWindow getter on the toolbox (or rely on the same logic in case the toolbox object is not available)

Assignee

Comment 1

2 months ago

Some classes in DevTools will not have an easy way to get access to the toolbox.
However they might still want to use the topmost chrome window.
Extract the logic from toolbox.js to a shared helper.

Assignee

Comment 2

2 months ago

Depends on D27672

Not strictly related to win.top/parent, but those tests would fail if the toolbox is in a content frame.
tooltip-01.js is creating a blank tab for no reason which prevents interacting with the actual test frame.
tooltip-02.js is not properly targeting an iframe to simulate a click

Assignee

Comment 4

2 months ago

Depends on D27675
The target of the iframe load event is the content document when running in frame with type=chrome.
When running in frame with type=content, the target will be the iframe element itself.
Stop relying on event.target so that the Sidebar can work in both cases

Assignee

Comment 5

2 months ago

Depends on D27677
Update tests directly referencing toolbox.win.top/parent.

Assignee

Comment 6

2 months ago

Try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ab3ba6dc624a606ba139a1df0744515b0712496

I tried to extract all the minor changes that could land regardless of the frame's type, and not related to one of bigger tracks (contextmenus, shortcuts or webextensions).

Assignee

Updated

2 months ago
Blocks: 1544709
Assignee

Comment 8

2 months ago

Will reorder the patch + leave-open to start landing things here.

Assignee

Updated

2 months ago
Keywords: leave-open

Comment 9

2 months ago
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/46afb0a8df4b
Add shared helper to retrieve the top window r=bgrins
https://hg.mozilla.org/integration/autoland/rev/ad0f4d97dd1e
Animation inspector scrubber should use getTopWindow r=daisuke
https://hg.mozilla.org/integration/autoland/rev/f71f9d2fa72b
Sidebar iframe onload callback should not rely on event.target r=daisuke
https://hg.mozilla.org/integration/autoland/rev/51423343879f
Use toolbox.topWindow in devtools tests relying on toolbox.win.top/parent r=daisuke

Comment 11

2 months ago
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/970ef36e6c09
Fix HTML Tooltip tests when running in content frame r=bgrins
Assignee

Updated

2 months ago
Keywords: leave-open

Comment 12

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
You need to log in before you can comment on or make changes to this bug.