295209 - Show details checkbox should use pref to persist across sessions

Status: RESOLVED WONTFIX

(Other Applications Graveyard :: Reporter, defect)

Description

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050522
Firefox/1.0+ ID:2005052223

open reporter and finish report
check the "show details" checkbox

open reporter again and fisish report
notice checkbox is unchecked

expected:
either use a button to show detail
or allways show deails (prefered and meening you can remove the option alltogether)
or make FF remember, which would require a pref

Comment 1

[Robert Accettura [:raccettura]]

we should add a pref

Comment 2

[Robert Accettura [:raccettura]]

Wrong bug to block... sorry for bugspam.

Comment 3

[Robert Accettura [:raccettura]]

Attachment: Patch v1

This should do it.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 208820 [details] [diff] [review]
Patch v1

>Index: reporter/resources/content/reporter/reportWizard.js

>+function setBoolPref(prefname, aValue) {

nit: aPrefName ? (oh, just noticed this is to match getBoolPref...)

>+function setShowDetail(){
>+  var hideDetail = getBoolPref("hideDetail", false);
>+  var hideDetailChecked = hideDetail ? false : true;

nit: hideDetailChecked  = !hideDetail ?

>+  document.getElementById('showDetail').checked = hideDetailChecked;
>+  document.getElementById('finishExtendedFrame').collapsed = hideDetail;
>+}

or maybe:
function setShowDetail() {
  var hideDetail = getBoolPref("hideDetail", false);
  document.getElementById('showDetail').checked = !hideDetail;
  document.getElementById('finishExtendedFrame').collapsed = hideDetail;
}
?

> function showDetail() {
>   var hideDetail = document.getElementById('showDetail').checked ? false : true;

This seems a lot clearer as: !document.getElementById('showDetail').checked , but maybe that's just me :)

>+  setBoolPref("hideDetail", hideDetail);
>   document.getElementById('finishExtendedFrame').collapsed = hideDetail;
> }

Maybe you could merge setShowDetail and showDetail into one? Perhaps:


function showDetail(aOnPageShow) {
  var hideDetail = getBoolPref("hideDetail", false);
  if (!aOnPageShow)
    document.getElementById('showDetail').checked = !hideDetail;
  document.getElementById('finishExtendedFrame').collapsed = hideDetail;
}

and then pass true from onPageShow and false (or nothing) from the checkbox itself?

Comment 5

[Robert Accettura [:raccettura]]

Attachment: Patch v2

Addressing comments.

FYI:
var hideDetail = document.getElementById('showDetail').checked ? false : true;

is all you.  You wrote that patch, I r= and checked in.

Now all of a sudden it's not up to your standards?  ;-)

Comment 6

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #5)
> FYI:
> var hideDetail = document.getElementById('showDetail').checked ? false : true;
> 
> is all you.  You wrote that patch, I r= and checked in.

All me? Gavin Sharp? I don't see my name anywhere in the blame for that ;).

Comment 7

[Robert Accettura [:raccettura]]

(In reply to comment #6)
> All me? Gavin Sharp? I don't see my name anywhere in the blame for that ;).
> 

No, sorry it's Mike's... he wrote the cleanup patch, and I checked it in.

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 208862 [details] [diff] [review]
Patch v2

Robert, clearing this request since it's ancient, not sure if it's still relevant/useful, if it is please re-request review. #mconnorisabadperson

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Reporter isn't a maintained project. Closing!