705131 - Implement Register Surface API in Highlighter

Status: RESOLVED WONTFIX

(DevTools :: Inspector, defect, P3)

Description

[Rob Campbell [:rc] (:robcee)]

We currently only have one display surface for the highlighter, but we have at least two more that we're going to land soon.

We need a lightweight API for registering new surfaces with the highlighter, and associated mechanisms in the highlighter (inspectorUI) for activating and deactivating them.

Comment 1

[Rob Campbell [:rc] (:robcee)]

in inspector.jsm:

/**
 * Register a new surface with the highlighter.
 * @param aSurface Represents a highlighter surface.
 *                 A surface should, at minimum support:
 *                   highlight()
 *                   hide()
 *                   isHidden()
 *                   show()
 * @param aInspectable Can the surface be used as a live inspector?
 * @param aOverlay Can the surface overlay another surface?
 */
registerSurface(aSurface, aInspectable, aOverlay):

activateSurface(aSurface)

deactivateSurface(aSurface)

Comment 2

[Victor Porof [:vporof][:vp]]

I thought about two more issues which should be solved by this API:

1. With Tilt, it was necessary to set the highlighterContainer display to "none" to avoid the Highlighter veil shown on top of the visualization canvas (after tab switches). I suppose the "aOverlay" flag takes care of this?

2. Also, it is was unfeasible with Tilt to close and restore state after tab switches, because initialization could take some time. Currently, the visualization is just hidden and re-shown when it has to. INSPECTOR_NOTIFICATIONS.CLOSED does not distinguish between these two cases afaict, thus a Surface (or other registered tool) can't know if the Inspector is really closed ("x" button or ESC key is pressed), or just the tabs are switched.

Another problem about this is: having an observer for INSPECTOR_CLOSED and tabs are switched, it will be fired *after* a TabSelect event. This way, a tool won't know for which tab INSPECTOR_CLOSED is fired, because gBrowser.selectedBrowser is already updated by the time the event is handled. A workaround would be for the tool to always remember the previous active tab, but this is very ugly.

Comment 3

[Victor Porof [:vporof][:vp]]

(In reply to Victor Porof from comment #2)
> I thought about two more issues which should be solved by this API:
>

Clarifications on issue 2.
Ultimately, INSPECTOR_NOTIFICATIONS.CLOSED should be distinguished in two ways:

a) location changes (pagehide events), or when "x" button, ESC or accesskey is pressed
b) TabSelect events

This way, registered Surfaces can avoid being forced to close and restore state, and thus rely only on INSPECTOR_NOTIFICATIONS for hiding/showing/closing.

Comment 4

[Victor Porof [:vporof][:vp]]

Second issue was fixed in bug 705731.

Comment 5

[Patrick Brosset <:pbro>]

The highlighter has changed so much in the past 3 years that this is invalid now.