900692 - Add paint flashing preference to Settings > Developer tools

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Paint flashing enabled

Paint flashing is controlled by nglayout.debug.paint_flashing. This can just be toggled on to enable paint flashing.

This would go in Settings > Developer tools, probably below the Remote debugging/learn more items.

Not sure what the right string for this would be:
Paint flashing []
Show repainting
Show repaints
Show tile repaints

Comment 1

[Chenxia Liu [:liuche]]

Attachment: Add paint flashing to Settings > Developer tools v1

Comment 2

[Lucas Rocha (:lucasr)]

Comment on attachment 817519 [details] [diff] [review]
Add paint flashing to Settings > Developer tools v1

Review of attachment 817519 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks good. But I wonder if we should add a description to the pref for extra clarity?

Comment 3

[Chenxia Liu [:liuche]]

Attachment: Screenshot: paint flashing setting

Ian, any feedback on this?

I'll see if I can add a short summary, something like "Flash tile repaints", "Show tile repaints as color flashing" (not sure about this, this might conflict with the Android style of updating the setting/preference summary to reflect the current state), or some other string.

Comment 4

[Chenxia Liu [:liuche]]

Attachment: Screenshot: summary text

This is just some summary text I threw on. We could also make it disappear when the checkbox is unchecked, though that might be slightly less helpful for figuring out what "Paint flashing" is.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Additional info on what this feature does:
https://developer.mozilla.org/en-US/docs/Tools/Paint_Flashing_Tool
http://msujaws.wordpress.com/2012/02/01/layout-paint-flashing-in-firefox/

Comment 6

[Ian Barlow (:ibarlow)]

(In reply to Chenxia Liu [:liuche] from comment #4)
> Created attachment 817996 [details]
> Screenshot: summary text
> 
> This is just some summary text I threw on. We could also make it disappear
> when the checkbox is unchecked, though that might be slightly less helpful
> for figuring out what "Paint flashing" is.

As you know, I know little to nothing about these kinds of developer tools, so I guess my question for you would be, do developers understand what paint flashing is? Or is that a unique tool to Mozilla? 

I'm looking through the core suite of desktop developer tools and don't see any mention of paint flashing, which suggests to me that we may want to offer an easy path to more info -- maybe some way to link out to one or both of the articles Mark posted?

Comment 7

[Ian Barlow (:ibarlow)]

Oh wait. I see it in the desktop dev tools now. Durr. 

Even still, I'm wondering if you feel it would be helpful to provide links out to more explanation?

Comment 8

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Learn more

I guess I just don't really like the way this looks - too much repetition?

I'll try to come up with a reasonable way that we could make the doc link look different, without 1) having it looked forced and out of place, and 2) not repeating the dev tool title itself (like "Paint flashing" and "Paint flashing docs"). Any input is welcome!

Comment 9

[Kevin Brosnan [Ex-Mozilla]]

Might be worth a relnote, cool feature for developers.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Kevin Brosnan [:kbrosnan] from comment #9)
> Might be worth a relnote, cool feature for developers.

If it had actually landed. Which reminds me, maybe we should try to land this...

I'd be OK with *not* using the "Learn more" for now and see how it is received with just the secondary text.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Please resubmit for the release notes once this bug is fixed. Thanks

Comment 12

[Chenxia Liu [:liuche]]

Attachment: Part 1: Add paint flashing (unbitrot)

Carrying over r+

Comment 13

[Chenxia Liu [:liuche]]

Attachment: Part 2: Tests

Added tests because there weren't any for this section at all! (oops)

https://tbpl.mozilla.org/?tree=Try&rev=479adfd3ea36

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8459015 [details] [diff] [review]
Part 2: Tests

Review of attachment 8459015 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm, assuming try is green.

Comment 15

[Chenxia Liu [:liuche]]

https://hg.mozilla.org/integration/fx-team/rev/96ae5120fa66
https://hg.mozilla.org/integration/fx-team/rev/8c9fbda3a4e9

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/96ae5120fa66
https://hg.mozilla.org/mozilla-central/rev/8c9fbda3a4e9

Comment 17

[Kevin Brosnan [Ex-Mozilla]]

Might be worth a relnote

Comment 18

[Chenxia Liu [:liuche]]

Relnote: Paint flashing for browser content repaints can now be enabled from Settings.

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Added with the wording "Developer tools: Paint flashing for browser content repaints".
Sorry, we try to remain consistent and to avoid sentences.