The markup view doesn't initialize until the page is fully loaded but it should be able to open on DOMContentLoaded

RESOLVED FIXED in Firefox 53

Status

defect
P2
normal
RESOLVED FIXED
4 years ago
10 months ago

People

(Reporter: bgrins, Assigned: ochameau)

Tracking

(Depends on 1 bug, Blocks 1 bug)

unspecified
Firefox 53
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [polish-backlog][difficulty=hard])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
This makes it feel sluggish on pages that take a long time for the 'load' event to fire.  We should be able to initialize on DOMContentLoaded instead.

To reproduce, open the inspector on a page like https://soundcloud.com/explore, then reload the page.  See how long it takes for the markup view to show back up.
(Reporter)

Updated

4 years ago
Summary: The markup view doesn't initialize until the page is fully loaded → The markup view doesn't initialize until the page is fully loaded but it should be able to open on DOMContentLoaded
Whiteboard: [devedition-40]
Whiteboard: [devedition-40] → [devedition-40][difficulty=hard]
Setting devedition-40 bugs to p3, filter on FB20EAC4-3FC3-48D9-B15F-0587C3987C25
Priority: -- → P3
(Reporter)

Updated

4 years ago
See Also: → 1036324
Whiteboard: [devedition-40][difficulty=hard] → [polish-backlog][difficulty=hard]
Blocks: 1208350
Duplicate of this bug: 1226154
(In reply to Brian Grinstead [:bgrins] from comment #2)
> The tab watcher in firebug might be useful:
> https://getfirebug.com/developer/api/firebug1.5/symbols/src/
> content_firebug_tabWatcher.js.html

The current code for this can be found here:
https://github.com/firebug/firebug/blob/master/extension/content/firebug/chrome/tabWatcher.js

Sebastian
(Reporter)

Updated

3 years ago
See Also: → 1105470
(Reporter)

Updated

3 years ago
See Also: → 1277348
(Reporter)

Updated

3 years ago
Duplicate of this bug: 1299238
Moving to P2 (filter on CLIMBING SHOES).
Priority: P3 → P2
(Assignee)

Comment 8

3 years ago
Bug 1277348 is going to fix one scenario. When opening the devtools against a still loading page, it will only wait for DOMContentLoaded instead of load.
But inspector is still going to wait for load event if you reload the page after the inspector is opened.
(Assignee)

Comment 11

3 years ago
We can listen for window-ready event from the TabActor, which is fired very early, once the Window object is ready, before any JS script from the page is executed and before DOMContentLoaded.
I already sent such patch to try and it seems to be fine, it just highlights one buggy test having a race:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c18d0713809b9d7cf2f88b8afd54711d3d44f93

I think it is worth testing that. You can see the DOM being created.
But it may also be CPU intensive and not that useful. It may be better to listen for window-ready, but then wait for the DOMContentLoaded event.
(Assignee)

Updated

3 years ago
Depends on: 1154645
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 17

3 years ago
Comment on attachment 8810419 [details]
Bug 1151909 - Wait for DOMContentLoaded

I kept this apart to allow you testing loading the markup view the earliest we can, before <body> is created. But I think we should start we loading the markup-view on DOMContentLoaded. It is less stressful for slow hardware.
We may always come back later to that, optimize the inspector performances and load on window-ready.

Note that it looks like Chrome starts loading its markup view before DOMContentLoaded as we see <body> being created.
(In reply to Alexandre Poirot [:ochameau] from comment #17)
> Comment on attachment 8810419 [details]
> Bug 1151909 - Wait for DOMContentLoaded
> 
> I kept this apart to allow you testing loading the markup view the earliest
> we can, before <body> is created. But I think we should start we loading the
> markup-view on DOMContentLoaded. It is less stressful for slow hardware.
> We may always come back later to that, optimize the inspector performances
> and load on window-ready.
> 
> Note that it looks like Chrome starts loading its markup view before
> DOMContentLoaded as we see <body> being created.
I've tested both approaches, and I think that waiting for DOMContentLoaded is a better solution.
Showing the DOM on window-ready is cool because the inspector is immediately showing something, but it's showing something that can't be used yet. Things update all the time, and might actually slow down the loading of the page. Also, the body node doesn't exist yet, so the inspector can't select it which is what it should normally do by default, and the rule-view is empty.
Waiting for DOMContentLoaded preserves that feature (body gets selected and the rule-view shows its rules). And content in the inspector appears way faster than it currently does in dev-edition.
So I vote for this solution.

Comment 19

3 years ago
mozreview-review
Comment on attachment 8808631 [details]
Bug 1151909 - Make the inspector actor wait for DOMContentLoaded instead of load.

https://reviewboard.mozilla.org/r/91428/#review93752

The code changes in this patch look good, but consider my comment in the bug about waiting for DOMContentLoaded.
Attachment #8808631 - Flags: review?(pbrosset) → review+

Comment 20

3 years ago
mozreview-review
Comment on attachment 8808632 [details]
Bug 1151909 - Fix browser_markup_load_01.js race by listening for events before executing the action that trigger them.

https://reviewboard.mozilla.org/r/91430/#review93754
Attachment #8808632 - Flags: review?(pbrosset) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8810419 - Attachment is obsolete: true
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Assignee: nobody → poirot.alex
(Assignee)

Comment 27

2 years ago
Looks like the markup view also need some tweaks to be also ready earlier on DOMContentLoaded.
With just the first two patches here is what you get:
* open a website
* open the inspector
* load another slow loading website like: lemonde.fr
* click on pick an element
* over elements
* you will see the highlighter change the selected element in the markup view without seeing the node highlighted on the website [<== this is the weird new behavior that doesn't exists today, as the markup view AND the highlighter are not working until load]
* once the website is fully loaded, elements starts being highlighted.
(Assignee)

Updated

2 years ago
Attachment #8813218 - Flags: review?(pbrosset)
(Assignee)

Comment 28

2 years ago
In that last patch I only modified the CanvasFrameAnonymousContentHelper.
There is still various highlighter code based on navigate but I don't know if that's any usefull to modify them? There is still highlighters.js, box-model and css-grid.

Comment 29

2 years ago
mozreview-review
Comment on attachment 8813218 [details]
Bug 1151909 - Make the highlighter work on DOMContentLoaded instead of load.

https://reviewboard.mozilla.org/r/94718/#review95548

> In that last patch I only modified the CanvasFrameAnonymousContentHelper.
> There is still various highlighter code based on navigate but I don't know if that's any usefull to modify them?
> There is still highlighters.js, box-model and css-grid.

Yeah, let's keep your patch simple like this and maybe filed bugs for the other highlighters you mentioned. :gl is doing work on the css-grid highlighter now, so it would be good to let him know about this required change in a new bug.
Attachment #8813218 - Flags: review?(pbrosset) → review+
(Assignee)

Comment 30

2 years ago
Looks like it may make devtools/client/inspector/test/browser_inspector_search-05.js even more intermittent:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d31c121abd9234ed01f8fbb105857ebb6c73ba62&selectedJob=31829177

I should push again with bug 1316238 to see if that fix it.
(Assignee)

Updated

2 years ago
Depends on: 1316238
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 35

2 years ago
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f137d9fa7ba
Make the inspector actor wait for DOMContentLoaded instead of load. r=pbro
https://hg.mozilla.org/integration/autoland/rev/c307daee56b5
Fix browser_markup_load_01.js race by listening for events before executing the action that trigger them. r=pbro
https://hg.mozilla.org/integration/autoland/rev/fb4f5c7e082a
Make the highlighter work on DOMContentLoaded instead of load. r=pbro

Comment 36

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7f137d9fa7ba
https://hg.mozilla.org/mozilla-central/rev/c307daee56b5
https://hg.mozilla.org/mozilla-central/rev/fb4f5c7e082a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
No longer blocks: top-inspector-bugs
Depends on: 1333711

Updated

2 years ago
Depends on: 1341756

Updated

2 years ago
Depends on: 1347761

Updated

2 years ago
Depends on: 1365075

Updated

2 years ago
Depends on: 1367372
Depends on: 1378133

Updated

2 years ago
Depends on: 1414597

Updated

10 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.