Closed
Bug 971798
Opened 10 years ago
Closed 10 years ago
Do not enable CSS reflow logging when clicking the CSS category
Categories
(DevTools :: Console, defect, P3)
DevTools
Console
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 31
People
(Reporter: msucan, Assigned: thomas)
Details
(Whiteboard: [mentor=msucan][lang=js,xul])
Attachments
(1 file, 1 obsolete file)
7.68 KB,
patch
|
msucan
:
review+
|
Details | Diff | Splinter Review |
It is too easy to enable CSS reflow logging by clicking the CSS category button in the web console. We should not auto-enable CSS > Log when the button is clicked. I also recommend we rename 'log' to 'reflow', since the CSS > Log menu item only does reflow logging.
Comment 1•10 years ago
|
||
Maybe it's time to move reflows logging somewhere else. But i'm not sure where.
Reporter | ||
Updated•10 years ago
|
Whiteboard: [mentor=msucan][lang=js,xul]
Assignee | ||
Comment 2•10 years ago
|
||
Mihai
If there is consensus for fixing this I would love to work on this one.
> I also recommend we rename 'log' to 'reflow'
Should it say 'reflows' (plural) like in Errors and Warnings?
Thomas
Reporter | ||
Comment 3•10 years ago
|
||
(In reply to Thomas Andersen from comment #2) > Mihai > If there is consensus for fixing this I would love to work on this one. Sure! Thanks! I want this bug fixed, irrespective of where CSS reflow logging will end up, because it's too easy to enable reflow listeners in the console, causing people to complain pages are slow. Until then, a patch to fix this bug is very much welcome. > > I also recommend we rename 'log' to 'reflow' > Should it say 'reflows' (plural) like in Errors and Warnings? Sure, makes sense.
Assignee: nobody → thomas
Status: NEW → ASSIGNED
Priority: -- → P3
Assignee | ||
Comment 4•10 years ago
|
||
Mihai This is an early preview patch. Should only fix the ui part of the issue. I am not sure if the logic should be in WebConsoleFrame._setMenuState, but I thought it was cleaner to have it as early as possible. Let me know if I miss anything. I think we should refactor code occurrences of 'csslog' to 'reflows' in webconsole.xul, webconsole.js, test/, browser/app/profile/firefox.js etc. too. If there are no upgrade scripts this would break the existing user preference after a browser upgrade. I guess the most critical issue regarding this would be add-ons/tools etc. that uses that pref. And of-course the browser/devtools/webconsole/test/browser_webconsole_bug_601667_filter_buttons.js test should be adjusted in order to pass now. Thomas
Attachment #8395201 -
Flags: review?(mihai.sucan)
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8395201 [details] [diff] [review] 971798.patch This is looking good. I dont think we need to update the prefs, severity, etc. Let's keep the patch minimal. Giving f+. We need the test fixes, then r+. Thanks for your patch!
Attachment #8395201 -
Flags: review?(mihai.sucan) → feedback+
Assignee | ||
Comment 6•10 years ago
|
||
ok, here is a new patch. I tried to make it as simple as possible ;) All webconsole mochittests passes for me now. thomas
Attachment #8395201 -
Attachment is obsolete: true
Attachment #8396042 -
Flags: review?(mihai.sucan)
Reporter | ||
Comment 7•10 years ago
|
||
Comment on attachment 8396042 [details] [diff] [review] 971798.patch Review of attachment 8396042 [details] [diff] [review]: ----------------------------------------------------------------- Thank you! Try push: https://tbpl.mozilla.org/?tree=Try&rev=c56baf6680e9 We will land the patch once the try results are green.
Attachment #8396042 -
Flags: review?(mihai.sucan) → review+
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/1746a5b54d12
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [mentor=msucan][lang=js,xul] → [mentor=msucan][lang=js,xul][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/1746a5b54d12
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [mentor=msucan][lang=js,xul][fixed-in-fx-team] → [mentor=msucan][lang=js,xul]
Target Milestone: --- → Firefox 31
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•