Closed Bug 993339 Opened 7 years ago Closed 7 years ago

The dialog with specific details doesn’t appears on Application tab for in-content preferences

Categories

(Firefox :: Preferences, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox29 --- wontfix
firefox30 --- affected
firefox31 --- affected

People

(Reporter: cbadau, Assigned: Paenglab)

References

Details

(Whiteboard: p=0 s=it-32c-31a-30b.1 [qa-])

Attachments

(1 file, 1 obsolete file)

Reproducible on the latest Nightly (BuildID: 20140407030203) 
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0
Reproducible on the latest Beta (BuildID:20140407135746)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20130205 Firefox/29.0
Reproducible on the latest Aurora (BuildID:20140407004002)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20130205 Firefox/30.0

Steps to reproduce: 
1. Open about:preferences. 
2. Go to Applications tab and select the "Application Details..." option for a chosen content type.

Actual results: The dialog with the specific details doesn't appear (like it does if we open Tools -> Options -> Applications) 

Expected results: The dialog with the specific details is correctly displayed (like it does if we open Tools -> Options -> Applications)

Notes: 1. This is reproducible also on Ubuntu x86 and Mac OSX 10.9. 
       2. This issue is not a regression.
Blocks: 738796
Flags: firefox-backlog?
Flags: firefox-backlog? → firefox-backlog+
document.documentElement.openSubDialog() doesn't work in in-content prefs. I use openDialog() instead.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8409352 - Flags: review?(gavin.sharp)
Why doesn't openSubDialog work?
Attachment #8409352 - Flags: review?(gavin.sharp) → review?(jaws)
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #2)
> Why doesn't openSubDialog work?

openSubDialog is defined on the prefwindow binding within toolkit/content/widgets/preferences.xml. Since the in-content preferences don't use prefwindow, reaching the documentElement and looking for openSubDialog will find that the function doesn't exist.
Comment on attachment 8409352 [details] [diff] [review]
inContentApplicationManager.patch

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

::: browser/components/preferences/in-content/applications.js
@@ +1689,5 @@
>  
> +    openDialog("chrome://browser/content/preferences/applicationManager.xul",
> +               "Browser:Application",
> +               "resizable=yes",
> +               handlerInfo);

Please use the same arguments that would have been used had this still been a call to openSubDialog. This means passing an empty string as the second argument and "modal,centerscreen,resizable=no" as the third argument.
Attachment #8409352 - Flags: review?(jaws) → review-
Review comment addressed and checked if the Cancel, Okay buttons are present on OS X.
Attachment #8409352 - Attachment is obsolete: true
Attachment #8412630 - Flags: review?(jaws)
Comment on attachment 8412630 [details] [diff] [review]
inContentApplicationManager.patch

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

Thanks!
Attachment #8412630 - Flags: review?(jaws) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/8ac8082f8029

FWIW, hg choked on this patch due to the ' in your commit message being a character it didn't like. Please can you try to avoid in the future? :). Also, keep in mind that in general, your commit message should be stating what the patch is actually doing, not restating the problem it's fixing.
https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment

And sorry for the empty cset you're going to see when this lands on m-c due to the aforementioned issue.
https://hg.mozilla.org/mozilla-central/rev/8ac8082f8029
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Hi Liz, can you review if this bug requires any QA verification.
Flags: needinfo?(lhenry)
Whiteboard: [qa?]
Whiteboard: [qa?] → p=0 s=it-32c-31a-30b.1 [qa?]
This doesn't need any QA verification.

Marco, what does setting p=0 on this resolved bug mean? Does p=___ have a use for QA costs separately from engineering costs?
Status: RESOLVED → VERIFIED
Flags: needinfo?(lhenry) → needinfo?(mmucci)
Thanks for the update Jared.  The 'p=0' tag refers to the point value we assign to bugs under development in a desktop iteration.  This bug was part of the Desktop Backlog and when it was resolved it was moved to the current iteration for a QA review, where applicable.
Flags: needinfo?(mmucci)
Whiteboard: p=0 s=it-32c-31a-30b.1 [qa?] → p=0 s=it-32c-31a-30b.1 [qa-]
(In reply to Marco Mucci [:MarcoM] from comment #11)
> Thanks for the update Jared.  The 'p=0' tag refers to the point value we
> assign to bugs under development in a desktop iteration.  This bug was part
> of the Desktop Backlog and when it was resolved it was moved to the current
> iteration for a QA review, where applicable.

Thanks for replying, but my confusion stemmed from the fact that this bug had already been marked as resolved-fixed, so adding p=0 after the fact is confusing. While there were no points assigned prior to the bug getting fixed, this was not a zero-effort bug fix and should have p=1 if the goal was to assign points after-the-fact.
Flags: needinfo?(mmucci)
Whiteboard: p=0 s=it-32c-31a-30b.1 [qa-] → p=1 s=it-32c-31a-30b.1 [qa-]
Flags: needinfo?(mmucci)
Whiteboard: p=1 s=it-32c-31a-30b.1 [qa-] → p=0 s=it-32c-31a-30b.1 [qa-]
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #12)
> Thanks for replying, but my confusion stemmed from the fact that this bug
> had already been marked as resolved-fixed, so adding p=0 after the fact is
> confusing. While there were no points assigned prior to the bug getting
> fixed, this was not a zero-effort bug fix and should have p=1 if the goal
> was to assign points after-the-fact.

Don't know if this was cleared up out-of-bug, but we're tracking points for bugs completed by "non-core" contributors differently. Since the participation of non-core contributors can be non-regular, it would skew the numbers and hurt predictability to include their work in the iteration stats. So we assign 0 points to those bugs (which isn't at all a reflection of their value!).

Going forward, there's no reason why we couldn't have regular volunteer contributors contribute to our "team velocity" and participate in the planning/status meetings, but it does require a certain level of time commitment which not all volunteers have.

Happy to take this to firefox-dev if you want to discuss it further, it's worth brainstorming some ways to improve this. For example, I think it would be good to estimate work anyways (i.e. assign the right p= value), but find other ways to filter/calculate the stats we want.
You need to log in before you can comment on or make changes to this bug.