Closed Bug 899701 Opened 6 years ago Closed 6 years ago

Expose chrome only paint flashing preference in all.js

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: milan, Assigned: milan)

References

Details

Attachments

(1 file, 1 obsolete file)

(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.
Attached patch Put the new preference in all.js (obsolete) — Splinter Review
Joe, could you also review my claims in comment 1?
Assignee: nobody → milan
Status: NEW → ASSIGNED
Attachment #783289 - Flags: review?(joe)
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 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.
Attachment #783289 - Flags: review?(joe) → review+
(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.
Summary: Expose chrome only paint flashing preference → Expose chrome only paint flashing preference in all.js
Fixed the comments, carrying r+ from Joe.
Attachment #783289 - Attachment is obsolete: true
Attachment #783341 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/22fd911b84ce
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.