Investigate a better highlighter API

ASSIGNED
Assigned to

Status

()

Firefox
Developer Tools: Inspector
P3
enhancement
ASSIGNED
27 days ago
6 days ago

People

(Reporter: jryans, Assigned: jryans)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox58 affected)

Details

(Whiteboard: [designer-tools])

User Story

Our highlighters currently use the `insertAnonymousContent` API to add the highlighter UI on top of content.

This approach has some downsides, such as:

* Very restricted[1] JS access to highlight UI, which leads to:
  * Features end up being built as a complex soup of attributes
  * Hard to test without DOM access to the markup
  * Hard to handle events from highlighter UI
* Can be challenging to correctly track content when APZ is involved (bug 1316318, bug 1312103)
* Hard to adapt the highlighter when the user zooms, scrolls, when elements are animated or transformed
* CSS on the page (for root element) may have an effect on the highlighter itself (bug 1379442)

There are other odds and ends as well:

* Highlighters don't work in SVG or XUL documents
* Nested frames are tricky to get right (although `getBoxQuads({relativeTo})` helps a lot)

A wish list from the DevTools side includes:

* Document-like solution that feels like normal HTML for the tool developer to support rapid iteration
* Access to all DOM APIs for highlighter UI while still being isolated from regular content to avoid style conflicts and script access by the page
* Mechanism to query info about specific content nodes and their ancestors, children, etc.
* Design should ensure the highlighter can follow rapidly changing content (animations) even when scrolling, APZ, etc. are involved
* Should be able to handle multiple boxes, lines, etc. on screen together with great performance
* Platform should calculate size / position of highlighters that track content during layout, rather than DevTools using `getBoxQuads` to achieve this manually
* Highlighter UI should be reachable via the inspector for debugging

[1]: http://searchfox.org/mozilla-central/source/dom/webidl/AnonymousContent.webidl
Comment hidden (empty)
(Assignee)

Comment 1

27 days ago
:pbro, :gl, you've each spent a lot of time with the current highlighter API.  Any additional downsides and / or wishlist items to suggest here?

I'd like to sync up with relevant platform groups (like APZ and graphics) to imagine a better API once we've collected our thoughts here.
Flags: needinfo?(pbrosset)
Flags: needinfo?(gl)
(Assignee)

Updated

27 days ago
Whiteboard: [designer-tools]
(Assignee)

Updated

27 days ago
User Story: (updated)
Some things that come to mind (some of which you already listed):

* it's hard to correctly adapt the highlighter when the user zooms, scrolls, when elements are animated or transformed,
* CSS on the page may have an effect on the highlighter itself (bug 1379442),
* highlighters are hard to test because accessing the generated markup is hard,
* we refrained ourselves from dynamically removing/adding to the anonymous content container when highlighters need to be updated, because they almost always need to be updated inside a requestAnimationFrame loop, so instead we generate all needed elements upfront and then play with attributes only to hide/show/manipulate things,
* this also means that for highlighters that require an unknown number of elements (like the css grid highlighter) we resorted to using a <canvas> element, making it hard to cover the whole page because of memory constraints,
* not having direct access to the highlighter's DOM nodes means that handling events is hard too (we need this for the css shapes editor for instance),
* highlighters do not work (yet) in SVG or XUL documents,
* it's hard to deal with nested frames too (although getBoxQuads({relativeTo}) helps a lot).

Note that before we introduced this API, we used to attach the highlighters in the browser XUL directly. This was before e10s and didn't support remote targets like FirefoxOS and Firefox for Android.

Last June I went to see Milan's team (graphics) during the Mozilla AllHands and we quickly talked about a different approach that would involve drawing highlighters during the rendering process, as a way to avoid a lot of these problems. Of course this would be very different from how we do things today, but worth considering. We filed bug 1377269. Here's an excerpt:

> It sounds like the big problems are that devtools need to recompute a lot of complex state that is already known to layout, 
> and that devtools overlays don't respond to APZ.
> The discussed solution was to add a DOM API that lets devtools request specific (hardcoded) overlays for given elements,
> and then platform can emit display items to draw the required overlay during normal display list building. This would let
> the overlay items pick up the same async scrolling metadata as the rest of the content for that element.
> They generally want the overlays to be on top of all web content, so we'd probably want to temporarily store an
> generated items somewhere (on the builder somewhere?) and then append them to the top of the final display list.
> They also want to override any clips set by content so that overlays can cover the whole page, but we need to make sure
> we don't escape the content area and into the UI (for non-e10s). Not entirely sure how that could work, but it seems feasible.
Flags: needinfo?(pbrosset)
I believe Chrome also uses a <canvas> for their highlighters, but they do test it.

I would personally prefer a true DOM solution for highlighters because this would allow very quick implementations of new highlighters. I have seen addons simply draw <div>s and apply css to replicate the grid lines. Also, this would be really simple to handle situations where we want grid line numbers to be sticky and always appear when scrolling. 

I would consider the developer ease of implementing, workflow and testing. This should also take into consideration our future requirements and performance. Typically, we need multiple infobars, multiple lines, multiple textboxes, and a way to query information about the current node and nodes (children, ancestor, siblings, etc) around it.
Flags: needinfo?(gl)
(Assignee)

Comment 4

13 days ago
Thanks all, I've updated the user story with your additional feedback.

:gl, when you said you need "a way to query information about the current node and nodes (children, ancestor, siblings, etc) around it", did you mean information about the nodes in the highlighter UI itself?  Or was this information about the picked content node(s) that the highlighter is trying to visualize?
User Story: (updated)
Flags: needinfo?(gl)
Content nodes that the highlighter is trying to visualize. 

I believe if we wanted to scale our highlighting and editing capabilities then we want a true DOM solution over <canvas> in this anonymous content. I would imagine there needs to be a pref mode where we can also inspect the anonymous content. This would be nice since we already style our anonymous content with css, and would add the ability to now inspect our anonymous content elements and why their styling might be off. 

I don't know how well the platform story will play out if we choose to write platform code for our highlighters. I am mostly comparing our current highlighter capabilities to how other addons including the first css grid highlighter addon was able to achieve their goals in a relatively small amount of code and time frame.
Flags: needinfo?(gl)
(Assignee)

Comment 6

13 days ago
(In reply to Gabriel [:gl] (ΦωΦ) from comment #5)
> Content nodes that the highlighter is trying to visualize.

Thanks, I've added that as well. 

> I don't know how well the platform story will play out if we choose to write
> platform code for our highlighters. I am mostly comparing our current
> highlighter capabilities to how other addons including the first css grid
> highlighter addon was able to achieve their goals in a relatively small
> amount of code and time frame.

I agree that a very low level solution in platform would make hard to iterate on the highlighters.  Thanks for emphasizing the importance of the developer workflow here, that's quite helpful!
User Story: (updated)
(Assignee)

Comment 7

13 days ago
For reference, here are some details about Chrome's current design.

Chrome uses a single overlay page[1] for anything DevTools might draw on top of content (debugger paused, rulers, grid highlighters).  This appears to be a full document layered on top of the page[2].  Most of the highlighter things use <canvas> (like Gabriel mentioned), but simpler things like element name, debugger paused, etc. use HTML and CSS.

The overlay page appears to have no access to content directly.  Instead, platform code invokes JS functions in the page[3] to enable various modes, passing JSON data to the overlay page as needed, with things like element quads, etc. calculated by the platform.  The overlay's platform agent code watches for content layout invalidation, schedules a redraw, and re-invokes overlay JS functions to update the highlighters.

The WebView for a browser tab can have several possible overlays, with the DevTools page overlay as one example.  When page layout is updated, the overlays are notified to recompute their own layout[4].

All of this takes appears to take place in the content process for the tab, assuming I am following Blink process boundaries correctly.

[1]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/inspector/InspectorOverlayPage.html?rcl=8edac08b78c89fe2895a99ca00d95bc42d94be03
[2]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/inspector/InspectorOverlayAgent.cpp?l=804&rcl=6b20dfea56f4ceb5894a678b457ad075879a79ac
[3]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/inspector/InspectorOverlayAgent.cpp?l=719&rcl=45e4c3b8ab4211c65e04e9354d61f0f26502ec6d
[4]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/exported/WebViewImpl.cpp?l=3486&rcl=6b20dfea56f4ceb5894a678b457ad075879a79ac
(Assignee)

Comment 8

12 days ago
More Chrome references, this time for testing.

It appears most highlighter tests[1] follow the pattern of:

1. Include some sample content
2. Ask the inspector agent in platform for the JSON data it would have passed to the overlay page
3. Compare the JSON with expected values

This JSON data is quite low-level[2], effectively serializing 2D <canvas> commands to run.

[1]: https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/tests/devtools/elements/highlight/
[2]: https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/tests/devtools/elements/highlight/highlight-css-grid-expected.txt
(Assignee)

Updated

10 days ago
User Story: (updated)
(Assignee)

Updated

6 days ago
Assignee: nobody → jryans
Status: NEW → ASSIGNED
You need to log in before you can comment on or make changes to this bug.