432212 - Viewing a Certificate from the Add Certificate Sheet Deactivates portions of the Window

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Stephen Horlander [:shorlander]]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050504 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050504 Minefield/3.0pre

When adding an exception for an invalid Certificate and then viewing the Certificate you are adding the browser looks inactive except for the titlebar and you must select another running application and go back to Firefox before it will go back to looking active.

Reproducible: Always

Steps to Reproduce:
1. Add an exception for a site with an invalid Certificate.
2. Get Certificate.
3. View Certificate and then close the resulting sheet.
4. Confirm exception.
Actual Results:  
Window goes to inactive look except for titlebar.

Expected Results:  
Window maintains active state appearance.

This looks like it only happens when you open a sheet from a sheet.

If you just add the exception without ever viewing the certificate the window behaves as expected.

Comment 1

[Stephen Horlander [:shorlander]]

Attachment: Example of problem

Comment 3

[Markus Stange [:mstange]]

Steven, could you have a look at it?
Something funny is going on there and I don't know how this case is supposed to be handled.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

> 1. Add an exception for a site with an invalid Certificate.

Please provide specific steps.

Comment 5

[Johnathan Nightingale [:johnath]]

(In reply to comment #4)
> > 1. Add an exception for a site with an invalid Certificate.
> 
> Please provide specific steps.

1a) Visit https://paypal.com/ (which uses a domain-mismatched certificate)
1b) Click "Or you can add an exception..."
1c) Click Add exception
2,3,4 as described

Comment 6

[Steven Michaud [:smichaud] (Retired)]

Thanks, johnath!

> 4. Confirm exception.

You don't need to confirm the exception -- canceling will also "work"
(will trigger the bug).

> This looks like it only happens when you open a sheet from a sheet.

This seems to be right.  Here's a much simpler testcase:

1) Open at least two tabs in a browser window.

2) "javascript:alert('blah');" in the window's location bar.

3) Cmd-q or Minefield : Quit Minefield

4) Cancel from the Quit Minefield dialog, then OK in the Javascript
   alert.

   The browser window now has the inactive appearance.

Tested in today's Minefield nightly on OS X 10.4.11.

I'll look into this further.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

Here's a patch that (in my tests with this bug's two STRs) fixes this
problem.

Turns out that [ChildView didEndSheet:returnCode:contextInfo:] always
re-activates the top-level non-sheet window.  But this is incorrect
behavior when the current widget's parent widget is also a sheet -- in
that case it's the parent sheet that should be re-activated (this
happens elsewhere).

Here's a tryserver build made with this patch:

https://build.mozilla.org/tryserver-builds/2008-05-06_10:[e-mail]-bugzilla432212/[e-mail]

Comment 8

[Stephen Horlander [:shorlander]]

Yeah your tryserver build fixes this for me.

Comment 9

[Josh Aas]

Comment on attachment 319619 [details] [diff] [review]
Fix

Great!

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 319619 [details] [diff] [review]
Fix

Though this shouldn't block the 1.9 release, I think it'd be good to
include it in 1.9.

This bug's patch fixes an issue related to bug 406730 (which _is_ a
blocker).  The patch is a well-defined fix for a well-defined problem
-- and therefore low risk.

Comment 11

[Mike Schroepfer]

Comment on attachment 319619 [details] [diff] [review]
Fix

a+ per triage today

Comment 12

[Josh Aas]

landed on trunk

Comment 13

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050804 Minefield/3.0pre as well as the equivalent nightly on Tiger. I used the STR in Comment 6 to verify.

Comment 14

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=5875 added to Litmus.