Closed Bug 1447490 Opened 2 years ago Closed Last year
Stop using gcli commands for all toolbox optional buttons
46 bytes, text/x-phabricator-request
|Details | Review|
A bunch of optional toolbox buttons are currently relying on gcli commands, whereas they could use regular actors to do that: * Paintflashing https://searchfox.org/mozilla-central/source/devtools/client/definitions.js#514 The real code that toggle the paint flashing, that should be moved to an actor is here: https://searchfox.org/mozilla-central/source/devtools/shared/gcli/commands/paintflashing.js#46-78 * Screenshot https://searchfox.org/mozilla-central/source/devtools/client/definitions.js#569 Relates to this whole module: https://searchfox.org/mozilla-central/source/devtools/shared/gcli/commands/screenshot.js#80 * rulers https://searchfox.org/mozilla-central/source/devtools/client/definitions.js#576 Relates to this piece of code: https://searchfox.org/mozilla-central/source/devtools/shared/gcli/commands/rulers.js#67-94 * measure https://searchfox.org/mozilla-central/source/devtools/client/definitions.js#592 Relates to a very similar piece of code to rulers, here: https://searchfox.org/mozilla-central/source/devtools/shared/gcli/commands/measure.js#69-97 Note that gcli handles a bunch of client/server state synchronization and manage to keep the menu checkbox in the right state. rulers and measure are both highlighters and may easily be converted to use: toolbox.highlighterUtils.getHighlighterByType("RulersHighlighter|MeasuringToolHighlighter").show/hide()
> * Screenshot About screenshot, this would need coordination with bug 1447491 as it also involves the screenshot command.
Priority: -- → P2
GCLI has been handling the state of our buttons as well as managing shared highlighters. Since the gcli removal, we no longer share the rulers and measure with any other UI, so a location for shared state is no longer needed.
try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ea10bdf62fd3a1feaae082b9a372f672c660c9f Patch takes care of ruler, measure, and paintflash. Screenshots button has been handled in #1483173
Attachment #9001596 - Attachment description: Bug 1447490 - Use highlighters instead of GCLI for rulers and measure; r=ochameau → Bug 1447490 - replace GCLI toolbox buttons with highlighters and appropriate actors; r=ochameau
Comment on attachment 9001596 [details] Bug 1447490 - replace GCLI toolbox buttons with highlighters and appropriate actors; r=ochameau Alexandre Poirot [:ochameau] has approved the revision.
Attachment #9001596 - Flags: review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/416ff8fc9760 replace GCLI toolbox buttons with highlighters and appropriate actors; r=ochameau
You need to log in before you can comment on or make changes to this bug.