Skip to content

Instantly share code, notes, and snippets.

@juliandescottes
Created February 12, 2020 15:35
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save juliandescottes/56d580cce7c2897e559c355db56e5f22 to your computer and use it in GitHub Desktop.
Save juliandescottes/56d580cce7c2897e559c355db56e5f22 to your computer and use it in GitHub Desktop.

Inspector initialization

Current mozilla central

constructor

  • [A.1] create the inspector store
  • [A.2] bind callbacks

init + deferredOpen (they could transparently be merged together)

  • [B.1] localize markup (modifies the DOM of the panel)
  • [B.2] watchTargets->onTargetAvailable
    • [B.2.1] initInspectorFront
    • [B.2.2] listen to "will-navigate"
    • [B.2.3] _getDefaultSelection (this will set this._defaultNode)
    • [B.2.4] _getAccessibilityFront (this will retrieve the target scope accessibility front)
    • [B.2.5] _getCssProperties (a bit convoluted, but initializes the target scoped cssProperties front)
    • NOT calling onNewRoot here, since isReady should be false
  • [B.3] create a StyleChangeTracker
    • this will end up calling watchTargets as well in walker-event-listener.
    • maybe we should make the constructor really dumb, and have an explicit init() call
  • [B.4] assign this._markupBox to the root element of the markup view
  • (now moving to deferredOpen)
  • [B.5] call initMarkup (without waiting on it to parallelize things a bit)
    • this mostly loads the markupview iframe, nothing specific to the target here
  • [B.6] selection.setNodeFront for this._defaultNode
    • remember this was retrieved during watchTarget/onTargetAvailable
  • [B.7] setupSplitter
    • nothing specific to the load or the target either, just build the markup
  • [B.8] toggle the visibility of inspector-main-content (to avoid seeing a UI "half ready")
  • [B.9] await setupSidebar
  • [B.10] now we wait for markup view iframe load (why now?)
  • [B.11] new HTMLBreadcrumbs
    • this will setup the class, create the markup and start displaying the current selection
    • should probably be split so that the constructor doesn't depend on the selection being ready
    • this way we could create HTMLBreadcrumbs early and just update when the selection is set
    • (which implies Breadcrumbs have to be ready before the selection is done, which honestly should be ok here. Worst case scenario, this means yet another watch like API for the selected node)
  • [B.12] setupExtensionSidebars:
    • not sure what is the expected contract when creating sidebars?
    • if they expect a valid selection, then this can't move much
  • [B.13] setupSearchBox
    • this is only initializing events and shortcuts
    • can be done earlier
  • [B.14] await setupToolbar
    • this one is "really" async because it needs to know if highlighters are supported in order to enable or not the eyedropper button. And this needs to be updated everytime we get a new root.
    • the other button of the toolbar (addNode) is updated everytime there is a selection, but without leveraging this.addNode.
    • I think this should be split so that the basic init is done early, and then the buttons get updated on selection/newRoot. The "setup" part would then become synchronous
  • [B.15] onNewSelection
    • now that everything should be initialized, simulate a new selection
    • remember we set the selection earlier, at the beginning of deferredOpen
  • [B.16] add a few event listeners
  • [B.17] record a telemetry scalar for 3 pane mode
  • [B.18] emit "ready"

The selection management seems to be one of the most problematic parts of this initialization. Selection is made in three steps for this first init:

  • retrieve the nodeFront for the default selection
  • set this node front on the selection singleton
  • trigger a fake "onNewSelection"

The reason we have this situation is that some widgets (eg Breadcrumbs) assume that a selection will be ready when they are built. Instead we should create all UI widgets, make sure they are all ready to react to the selection, and update the selection at the end. We might still want to retrieve the selected node front earlier (anytime after new root is ready) to parallelize the async work done during the init.

onNewRoot

  • [C.1] nullify the selection
  • [C.2] destroy the markupview
  • [C.3] get the defaultNodeFront
    • should be ok to do because we have a ready-to-use walker with a valid root node
    • more or less the same as [B.2.3]. If we make onTargetAvailable rely consistently on onNewRoot, we will be able to drop [B.2.3]
  • [C.4] selection.setNodeFront
    • more or less the same as [B.6], but the reason passed to setNodeFront is different ("navigateaway" vs "inspector-open"). However both reasons have the same role in the rest of the code base. They are in the list of selection reasons which are ignored by the markupview (because the markupview should be reinitialized shortly after in both cases)
  • [C.5] call this._initMarkup()
    • same as [B.5]
  • [C.6] call setupToolbar()
    • same as [B.14]
  • [C.7] when "markuploaded" is received, call this.onMarkupLoaded
    • [C.7.1] markup.expandNode(this.selection.nodeFront)
      • this is only done here. This is not making the selected node visible in the markup. On the contrary this assumes the selected node is already properly loaded and it will simply expand the container.
      • if you go to a page, open the inspector, select a node that has children but keep it collapsed, and reload the page... the node will be expanded after reloaded. This seems inconsistent and hard to justify?
    • [C.7.2] restore various highlighter states
    • [C.7.3] emit new-root
    • [C.7.4] wait for expandNode (C.7.1) to resolve.
    • [C.7.5] emit reloaded
      • I have seen tests where we wait for reloaded because other wise there are pending requests etc... but those additional requests come from the expandNode call. Really think we should try to get rid of that.
    • [C.7.6] submit new-root/reload time to telemetry

_onBeforeNavigate

  • this._defaultNode = null
  • this.selection.setNodeFront(null)
  • this._destroyMarkup()
  • this._pendingSelection = null
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment