899701 - Expose chrome only paint flashing preference in all.js

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

Bug 895003 added chrome only paint flashing support.  This bug exposes it explicitly in the prefs and some debugging places.  See bug 895003 comment 18:

Do we need follow-up bugs for:
http://mxr.mozilla.org/comm-central/source/mozilla/widget/xpwidgets/nsBaseWidget.cpp#1617
http://mxr.mozilla.org/comm-central/source/mozilla/layout/tools/layout-debug/src/nsLayoutDebuggingTools.cpp#115

Perhaps this as well?
http://mxr.mozilla.org/comm-central/source/mozilla/modules/libpref/src/init/all.js#476

Not sure what this one is about:
http://mxr.mozilla.org/comm-central/source/mozilla/b2g/chrome/content/settings.js#286

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Milan Sreckovic [:milan] from comment #0)
> Bug 895003 added chrome only paint flashing support.  This bug exposes it
> explicitly in the prefs and some debugging places.  See bug 895003 comment
> 18:
> 
> Do we need follow-up bugs for:
> http://mxr.mozilla.org/comm-central/source/mozilla/widget/xpwidgets/
> nsBaseWidget.cpp#1617

Don't think we need this.

> http://mxr.mozilla.org/comm-central/source/mozilla/layout/tools/layout-debug/
> src/nsLayoutDebuggingTools.cpp#115
> 

Don't think we need this.

> Perhaps this as well?
> http://mxr.mozilla.org/comm-central/source/mozilla/modules/libpref/src/init/
> all.js#476

Yes we need this, otherwise people need to know the name to use it.

> 
> Not sure what this one is about:
> http://mxr.mozilla.org/comm-central/source/mozilla/b2g/chrome/content/
> settings.js#286

Don't think we need this.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Put the new preference in all.js

Joe, could you also review my claims in comment 1?

Comment 3

[Joe Drew (not getting mail)]

It's needed in all.js and the b2g chrome js (the latter because otherwise the chrome won't get the setting when content changes it, AIUI). Don't put it in the layout debugger or debug_ nsBaseWidget; that needs more work to work, and could probably have a followup bug filed.

Comment 4

[Joe Drew (not getting mail)]

Comment on attachment 783289 [details] [diff] [review]
Put the new preference in all.js

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

::: modules/libpref/src/init/all.js
@@ +474,5 @@
>  
>  // whether or not to draw images while dragging
>  pref("nglayout.enable_drag_images", true);
>  
> +// enable/disable paint flashing --- useful for debugging (the second one is for chrome only)

This comment could clarify whether paint_flashing applies also to chrome.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Joe Drew (:JOEDREW! \o/) from comment #3)
> It's needed in all.js and the b2g chrome js (the latter because otherwise
> the chrome won't get the setting when content changes it, AIUI).

I think making the B2G change is a larger change as well.  I should probably change the summary of this bug to just reflect the all.js nature of the fix.  To make this into a separate option on B2G is not hard, but it touches more things.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Add the pref to all.js, carry r=joe.

Fixed the comments, carrying r+ from Joe.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/22fd911b84ce

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/22fd911b84ce