Closed Bug 647365 Opened 13 years ago Closed 13 years ago

Clicking on "full screen" crashes Camino while watching Netflix streaming; just installed Mac OS 10.6.7.

Categories

(Camino Graveyard :: Plug-ins, defect)

x86
macOS
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Lon, Assigned: stuart.morgan+bugzilla)

References

()

Details

(Whiteboard: [camino-2.0.9])

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en; rv:1.9.0.19) Gecko/2011032020 Camino/2.0.7 (like Firefox/3.0.19)
Build Identifier: Version 2.0.7 (1.9.0.19 2011032020)

Tried several times;  always got the same result.  Mac OS 10.6.7 is the only major change today, and it worked last night.

Reproducible: Always

Steps to Reproduce:
1.Open Netflix.com
2.Start movie
3.Click on full screen
4.Crash



Move to full screen.
Can you provide a crash report ?
See http://caminobrowser.org/documentation/bugzilla/#crash for a how-to.
So I don't know why it's crashing, but it's definitely us telling the plugin window to redraw, and that window is crashing on draw. We should just skip windows we didn't create when running this code.

I'm actually wondering if it's really necessary though. That code is quite old:
https://bugzilla.mozilla.org/show_bug.cgi?id=153651
so before I make it more complicated I'll test to see if it's actually still necessary.

(Unfortunately it doesn't say what the symptoms were supposed to be; hopefully it's just the obvious 'colors aren't right any more')
Assignee: nobody → stuart.morgan+bugzilla
Component: General → Plug-ins
Flags: camino2.1b1?
Flags: camino2.0.8?
QA Contact: general → plugins
Hardware: Other → x86
Apparently newer OS versions (and/or machines?) no longer have the ability to change color depth, so I can't actually test this :P

Smokey, do you have a machine that can change bit depth, to test a build with that code removed?
Attached patch uglier fix (obsolete) — Splinter Review
This is the fix I know won't regress anything. I'll hold off on landing until we see about testig though, since I'd much rather remove the whole function.
(In reply to comment #4)
> Apparently newer OS versions (and/or machines?) no longer have the ability to
> change color depth, so I can't actually test this :P
> 
> Smokey, do you have a machine that can change bit depth, to test a build with
> that code removed?

Yeah, it looks like I can switch to thousands on 10.5 on my MBP.  I won't be able to test anything until next week, likely.
Might as well confirm this since there's something we can patch.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attached patch v2Splinter Review
Helps if I take out the code I'm replacing, instead of just adding the new code :P

I'll just go ahead and land this. Figuring out if we can remove this code isn't a priority.
Attachment #523830 - Attachment is obsolete: true
Attachment #531606 - Flags: superreview?
http://hg.mozilla.org/camino/rev/4a8f1e3d754a landed. Once we have a build to test and can verify it's fixed, we can merge to branch.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: camino2.1b1? → camino2.1b1+
Resolution: --- → FIXED
Attachment #531606 - Flags: superreview? → superreview?(mikepinkerton)
(In reply to Stuart Morgan from comment #9)
> http://hg.mozilla.org/camino/rev/4a8f1e3d754a landed. Once we have a build
> to test and can verify it's fixed, we can merge to branch.

What's the test here?

(And what was the code+test I was supposed to do in comment 4?)
The test is to change color depth as described in bug 153651 and see what happens (e.g., if you are looking at a photo, does it become ugly, then pretty again? I'm hoping the difference would be visible to the naked eye...).

The comment 4 test would be doing the same after completely removing the whole applicationDidChangeScreenParameters: method.
(In reply to Stuart Morgan from comment #11)
> The test is to change color depth as described in bug 153651 and see what
> happens (e.g., if you are looking at a photo, does it become ugly, then
> pretty again? I'm hoping the difference would be visible to the naked
> eye...).

In current 2.1 builds, looking at both a photo and http://caminobrowser.org/, my whole screen fades to blue and then fades back, and I notice nothing obvious (other than some UI elements, like the toolbar and window shadow, are noticeably less smooth but not garishly ugly, which is what I'd expect trying to display things designed for a millions world in a thousands display setting).

> The comment 4 test would be doing the same after completely removing the
> whole applicationDidChangeScreenParameters: method.

With that whole method removed, the behavior remains the same as in shipping 2.1 builds above.

I'm not sure what that all means; can someone find us a time machine back to 2002? ;-)
Sounds like we can probably rip out the whole method then. Yay!
We can take this (or an entire removal) for 2.0.9, then, if we get there, I guess.
Flags: camino2.0.8? → camino2.0.8-
CAMINO_2_0_BRANCH; for 2.0.9 I just landed the same patch that's already been landed for 2.1.
Flags: camino2.0.9? → camino2.0.9+
Whiteboard: [camino-2.0.9]
Attachment #531606 - Flags: superreview?(mikepinkerton)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: