Last Comment Bug 689946 - Docked HTML panel in the highlighter should remember its height
: Docked HTML panel in the highlighter should remember its height
Status: VERIFIED FIXED
[has-patch][fixed-in-fx-team][testday...
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: Firefox 10
Assigned To: Rob Campbell [:rc] (:robcee)
:
Mentors:
Depends on:
Blocks: 663830
  Show dependency treegraph
 
Reported: 2011-09-28 07:42 PDT by Rob Campbell [:rc] (:robcee)
Modified: 2011-11-25 06:05 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
persist (1.59 KB, patch)
2011-11-05 16:40 PDT, Rob Campbell [:rc] (:robcee)
no flags Details | Diff | Splinter Review
Persist (2.12 KB, patch)
2011-11-07 06:16 PST, Rob Campbell [:rc] (:robcee)
no flags Details | Diff | Splinter Review
persist with test (7.34 KB, patch)
2011-11-07 08:49 PST, Rob Campbell [:rc] (:robcee)
mihai.sucan: review+
Details | Diff | Splinter Review

Description Rob Campbell [:rc] (:robcee) 2011-09-28 07:42:39 PDT
The docked HTML panel should be able to remember its height when resized using the toolbar resizer. It should save its size to a preference named something like:

devtools.highlighter.html.height.
Comment 1 Dave Camp (:dcamp) 2011-10-27 08:51:32 PDT
We're doing developer tool prioritization, filter on 'brontozaur'
to ignore the spam.
Comment 2 Rob Campbell [:rc] (:robcee) 2011-11-05 16:40:19 PDT
Created attachment 572239 [details] [diff] [review]
persist
Comment 3 Rob Campbell [:rc] (:robcee) 2011-11-07 05:21:44 PST
Comment on attachment 572239 [details] [diff] [review]
persist

still needs a test, but a round of feedback while I write it couldn't hurt.
Comment 4 Paul Rouget [:paul] 2011-11-07 05:29:38 PST
Where do 112 and 64 come from?
Comment 5 Rob Campbell [:rc] (:robcee) 2011-11-07 06:15:20 PST
(In reply to Paul Rouget [:paul] from comment #4)
> Where do 112 and 64 come from?

no idea.
Comment 6 Rob Campbell [:rc] (:robcee) 2011-11-07 06:16:12 PST
Created attachment 572446 [details] [diff] [review]
Persist
Comment 7 Mihai Sucan [:msucan] 2011-11-07 08:23:31 PST
Comment on attachment 572446 [details] [diff] [review]
Persist

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

I think you forgot to do qref. This patch is missing the test file. Make also fails.

The HTML panel height is hard-coded. Why not do window.innerHeight * 0.2 or something?
Comment 8 Rob Campbell [:rc] (:robcee) 2011-11-07 08:28:11 PST
Comment on attachment 572446 [details] [diff] [review]
Persist

crap, forgot to hg add the file. standby...
Comment 9 Rob Campbell [:rc] (:robcee) 2011-11-07 08:49:17 PST
Created attachment 572483 [details] [diff] [review]
persist with test

here we go.
Comment 10 Mihai Sucan [:msucan] 2011-11-07 09:21:39 PST
Comment on attachment 572483 [details] [diff] [review]
persist with test

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

Patch looks good, but I do have some nits (non-binding for the review):

- the default panel height is still hard-coded.
- there's trailing whitespace.
- test license should be the PD boilerplate.
- test doc.body.innerHTML is heavy - not needed for this kind of test.
- arguments.callee should no longer be used.
Comment 11 Rob Campbell [:rc] (:robcee) 2011-11-07 11:34:50 PST
https://hg.mozilla.org/integration/fx-team/rev/7c054e3df274
Comment 12 Rob Campbell [:rc] (:robcee) 2011-11-07 15:05:37 PST
https://hg.mozilla.org/mozilla-central/rev/7c054e3df274
Comment 13 Ioan C. 2011-11-25 06:03:40 PST
Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111123 Firefox/10.0a2
Verified fixed. http://screencast.com/t/YLYTA0lq

Note You need to log in before you can comment on or make changes to this bug.