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

NEW
Unassigned

Status

defect
P1
normal
18 years ago
6 years ago

People

(Reporter: kmcclusk, Unassigned)

Tracking

Trunk
Future
PowerPC
macOS
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: Fix described, URL)

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
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.
(Reporter)

Comment 1

18 years ago
Reassigning to Pierre since Don is on Sabbatical.
Assignee: kmcclusk → pierre

Comment 2

18 years ago
For more info, see the URL above or go to http://www.apple.com/developer/ and 
look for 'kAEDisplayNotice'
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.6
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.
(Reporter)

Updated

18 years ago
Blocks: 103889

Updated

18 years ago
Depends on: 6061

Comment 4

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

Updated

18 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.8

Updated

18 years ago
Whiteboard: Fix described

Comment 5

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

Comment 6

18 years ago
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.
Priority: -- → P3
Summary: Mac needs to generate NS_DISPLAYCHANGED event when the display depth changes → [review]Mac needs to generate NS_DISPLAYCHANGED event when the display depth changes

Comment 7

18 years ago
Posted patch patchSplinter Review
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.
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.

Updated

18 years ago
Target Milestone: mozilla0.9.8 → mozilla0.9.9
(Reporter)

Comment 10

18 years ago
Marking nsbeta1+.  Pierre can you make the changes suggested by the reviewers?
Keywords: nsbeta1+
(Reporter)

Comment 11

17 years ago
Reassigning to Don.
Assignee: pierre → dcone
Status: ASSIGNED → NEW
Priority: P3 → P2
Target Milestone: mozilla0.9.9 → mozilla1.0

Comment 12

17 years ago
nsbeta1- per adt triage
Keywords: nsbeta1+nsbeta1-
Target Milestone: mozilla1.0 → ---
(Reporter)

Updated

17 years ago
Priority: P2 → P1
Target Milestone: --- → Future
Taking
Assignee: dcone → sfraser
Target Milestone: Future → mozilla1.1alpha

Comment 14

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

Updated

17 years ago
Keywords: review

Updated

16 years ago
Component: GFX → GFX: Mac

Comment 15

16 years ago
Mozilla CFM build is dead; should this bug go with it?
Still applies to OS X.
OS: Mac System 9.x → MacOS X
Target Milestone: mozilla1.1alpha → Future
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.
Assignee: sfraser_bugs → joshmoz
(Assignee)

Updated

10 years ago
Product: Core → Core Graveyard

Updated

10 years ago
Assignee: joshmoz → nobody
You need to log in before you can comment on or make changes to this bug.