Add paint flashing preference to Settings > Developer tools

RESOLVED FIXED in Firefox 33

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: liuche, Assigned: liuche)

Tracking

({feature})

unspecified
Firefox 33
ARM
Android
feature
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(relnote-firefox 33+)

Details

Attachments

(6 attachments, 1 obsolete attachment)

Created attachment 784594 [details]
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
Created attachment 817519 [details] [diff] [review]
Add paint flashing to Settings > Developer tools v1
Attachment #817519 - Flags: review?(lucasr.at.mozilla)
Assignee: nobody → liuche
Status: NEW → ASSIGNED
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?
Attachment #817519 - Flags: review?(lucasr.at.mozilla) → review+
Created attachment 817949 [details]
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.
Flags: needinfo?(ibarlow)
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.
(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?
Flags: needinfo?(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?
Created attachment 818540 [details]
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!
Flags: needinfo?(ibarlow)
Flags: needinfo?(ibarlow)
Might be worth a relnote, cool feature for developers.
relnote-firefox: --- → ?
Keywords: feature
(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.
Please resubmit for the release notes once this bug is fixed. Thanks
relnote-firefox: ? → ---
Created attachment 8459014 [details] [diff] [review]
Part 1: Add paint flashing (unbitrot)

Carrying over r+
Attachment #8459014 - Flags: review+
Created attachment 8459015 [details] [diff] [review]
Part 2: Tests

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

https://tbpl.mozilla.org/?tree=Try&rev=479adfd3ea36
Attachment #8459015 - Flags: review?(margaret.leibovic)
Attachment #817519 - Attachment is obsolete: true
Comment on attachment 8459015 [details] [diff] [review]
Part 2: Tests

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

lgtm, assuming try is green.
Attachment #8459015 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/96ae5120fa66
https://hg.mozilla.org/mozilla-central/rev/8c9fbda3a4e9
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Might be worth a relnote
relnote-firefox: --- → ?
Relnote: Paint flashing for browser content repaints can now be enabled from Settings.
Added with the wording "Developer tools: Paint flashing for browser content repaints".
Sorry, we try to remain consistent and to avoid sentences.
relnote-firefox: ? → 33+
You need to log in before you can comment on or make changes to this bug.