101843 - [review]Mac needs to generate NS_DISPLAYCHANGED event when the display depth changes

Status: RESOLVED INCOMPLETE

(Core Graveyard :: GFX: Mac, defect, P1)

Description

[Kevin McCluskey (gone)]

This bug was split off from bug 6061. Changing bit depth/# of colors busts
chrome, images

On Mac we need to add a method to nsWindow which is called when the display has
changed:

PRBool nsWindow::ReportDisplayChange()
{
	// nsEvent
	nsGUIEvent changeEvent;
	changeEvent.eventStructType = NS_GUI_EVENT;
	changeEvent.message = NS_DISPLAYCHANGED;
	changeEvent.time = PR_IntervalNow();

	// nsGUIEvent
	changeEvent.widget		= this;
	changeEvent.nativeMsg		= nsnull;

	// dispatch event
	return (DispatchWindowEvent(changeEvent));
}

The call to DisplayWindowEvent will take care of getting the message to the
viewmanager.

Comment 1

[Kevin McCluskey (gone)]

Reassigning to Pierre since Don is on Sabbatical.

Comment 2

[Pierre Saslawsky]

For more info, see the URL above or go to http://www.apple.com/developer/ and 
look for 'kAEDisplayNotice'

Comment 3

[Simon Fraser [no longer active]]

Ref is here:
http://developer.apple.com/techpubs/macosx/Carbon/graphics/DisplayManager/
Optimizing_DisplayManager/Conceptual/DisplayManager-7.html

Note that you have to set a bit in your 'SIZE' flags to get send this event.

Comment 4

[Greg K.]

*** Bug 106238 has been marked as a duplicate of this bug. ***

Comment 5

[R.K.Aa.]

*** Bug 114549 has been marked as a duplicate of this bug. ***

Comment 6

[Pierre Saslawsky]

pink/sfraser: please r/sr

Simon, a while back we exchanged the following emails...
----
> I see 2 ways of fixing this:
> [...]
> 2) COMify nsMacMessageSink to export IsRaptorWindow() and DispatchOSEvent()
> and use these in an event handler that I register in AppRunner through the
> functions you wrote...

I think it would be more appropriate for some service to have
an API that we can call that tells each window to send the 
NS_DISPLAYCHANGED event. nsIWindowWatcher looks like a good
candidate, and is already called from the AE code.
----

The code changed quite a bit in the meanwhile and it allowed me to implement a 
patch all in the XPFE AppleEvents code.  Another reason was that I did not want 
to modify some XP interfaces to implement functions that would only be used on 
the Mac.  Look at the attached patch: I'm not sure you are going to like it but 
it works fine and we may even have to reuse it to propagate other events.

Comment 7

[Pierre Saslawsky]

Attachment: patch

Comment 8

[Simon Fraser [no longer active]]

Comments:

I don't think nsAEGenericClass is the correct place for the display-changed 
handling code. Since this is an application-level event, it should live in 
nsAEApplicationClass.cpp.

Also, I'm trying to keep the AppleEvent code as free as possible of real code; it 
should really just make a high-level call into some other class that does the 
real work. So I'd rather DispatchGUIEvent() and the guts of 
HandleSystemConfigNotice() move elsewhere.

+        nsISupports* child;
+        if (NS_SUCCEEDED(children->CurrentItem(&child))) {

nsCOMPtr?

+          nsIWidget* childWidget = static_cast<nsIWidget*>(child);

QueryInterface?

+          DispatchGUIEvent(childWidget, anEvent, PR_TRUE);

This this recurses, I'd like to see a little paranoia; maybe check that 
childWidget != aWidget.

Comment 9

[Mike Pinkerton (not reading bugmail)]

comments:

1) i don't like they typecast to nsIBidirectionalEnumerator on:

+    nsCOMPtr<nsIBidirectionalEnumerator>
children(getter_AddRefs((nsIBidirectionalEnumerator*)aWidget->GetChildren()));

it makes an assumption that the enumerator returned from GetChildren() is bidi,
when the implementation may not be at all. Use QueryInterface to move between
interfaces, not casts.

2) Again, same complaint here:

+          nsIWidget* childWidget = static_cast<nsIWidget*>(child);

These assumptions _might_ be ok within the widget code (i still don't like
them), but certainly not in xpfe/bootstrap.

Comment 10

[Kevin McCluskey (gone)]

Marking nsbeta1+.  Pierre can you make the changes suggested by the reviewers?

Comment 11

[Kevin McCluskey (gone)]

Reassigning to Don.

Comment 12

[Syd Logan]

nsbeta1- per adt triage

Comment 13

[Simon Fraser [no longer active]]

Taking

Comment 14

[Greg K.]

*** Bug 124236 has been marked as a duplicate of this bug. ***

Comment 15

[Frankie]

Mozilla CFM build is dead; should this bug go with it?

Comment 16

[Simon Fraser [no longer active]]

Still applies to OS X.

Comment 17

[Dave Miller [:justdave]]

The docs listed above are outdated (and 404)

Closest thing I can find surfing around developer.apple.com is here:

http://developer.apple.com/documentation/Carbon/Reference/Display_Manager/displaymgr/function_group_5.html

Which isn't an AppleEvent, but might be a more direct way to do it.