Closed Bug 1447490 Opened 2 years ago Closed Last year

Stop using gcli commands for all toolbox optional buttons

Categories

(DevTools Graveyard :: Graphic Commandline and Toolbar, enhancement, P2)

enhancement

Tracking

(firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: ochameau, Assigned: yulia)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

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.
Product: Firefox → DevTools
Depends on: 1483173
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.
Assignee: nobody → ystartsev
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
Depends on: 1485022
Blocks: 1486799
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+
Blocks: 1487442
Pushed by ystartsev@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/416ff8fc9760
replace GCLI toolbox buttons with highlighters and appropriate actors; r=ochameau
https://hg.mozilla.org/mozilla-central/rev/416ff8fc9760
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.