Last Comment Bug 518915 - NPAPI claims NPWindow::clipRect is only used on Mac, but that's not true
: NPAPI claims NPWindow::clipRect is only used on Mac, but that's not true
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla1.9.3a1
Assigned To: Karl Tomlinson (:karlt)
:
Mentors:
Depends on:
Blocks: 568219
  Show dependency treegraph
 
Reported: 2009-09-25 13:46 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-02-06 18:08 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
delete comment (925 bytes, patch)
2009-09-29 11:09 PDT, Karl Tomlinson (:karlt)
jaas: review+
Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2009-09-25 13:46:07 PDT
It's at least also used for X11.

Fixing this just entails a minor edit or deletion of this comment in npapi.h.
Comment 1 Karl Tomlinson (:karlt) 2009-09-29 11:09:55 PDT
Created attachment 403518 [details] [diff] [review]
delete comment

The comment also needs deleting from https://developer.mozilla.org/en/NPWindow, though the description of the argument there seems right.
https://developer.mozilla.org/en/Gecko_Plugin_API_Reference:Drawing_and_Event_Handling is also ok.

In Mozilla code, clipRect is set correctly for X11 plugins (though only really needed for windowless plugins).  For windowless win32 plugins, its not small enough to be effective, so the plugin actually needs to confine drawing to the dirty rect.

clipRect seems to be set correctly for Mac plugins (though there are too many different code paths for me to really know).  Looks like plugins with NPEventModelCocoa and NPDrawingModelCoreGraphics also get a dirty rect.

The story seems too difficult to tell in npapi.h, and I'm not really sure whether the windowless win32 issue is a bug in Mozilla or plugins are just expected to confine drawing to the "update area" in the WM_PAINT message, so removing the comment seems best.
Comment 2 Karl Tomlinson (:karlt) 2009-10-13 20:36:32 PDT
http://hg.mozilla.org/mozilla-central/rev/312699f3738b

Thanks, Josh for updating https://developer.mozilla.org/en/NPWindow.
Comment 3 Jim Mathies [:jimm] 2010-06-02 15:09:55 PDT
Reopening to back this change out. 'clipRect' isn't needed on other platforms, so there's no need to indicate it's valid other platforms. Also, it's not supported in our current ipc shim code for win32.
Comment 4 Karl Tomlinson (:karlt) 2010-06-02 15:20:11 PDT
clipRect is needed on X11.
Comment 5 Jim Mathies [:jimm] 2010-06-02 15:29:41 PDT
(In reply to comment #4)
> clipRect is needed on X11.

Ok, so I pulled the old patch -

http://hg.mozilla.org/mozilla-central/rev/8e30dc6ebc53

I guess we should update again to state that this is valid on X11 and OSX?
Comment 6 Karl Tomlinson (:karlt) 2010-06-02 15:56:10 PDT
(In reply to comment #5)
Why you are asking that question?
Why did you add a comment that is wrong?
Comment 7 Jim Mathies [:jimm] 2010-06-02 17:18:49 PDT
(In reply to comment #6)
> (In reply to comment #5)
> Why you are asking that question?
> Why did you add a comment that is wrong?

I just pulled the original patch. We can touch it up or reland or whatever, we just need to figure out what we're going to do here. 

There's a patch in bug 568219 that makes this work on win32 with oopp. W/out oopp the values were already set in nsObjectFrame.

I really don't feel strongly one way or another.

I would add that when I did the oopp work, I did a bit of searching and came across a lot of docs/code/whatever that had that "mac only" comment in them, hence the reason why oopp on win32 doesn't support clipRect currently. So I doubt any plugins on win32 are currently using this, and I don't believe they need it either. (it's always 0,0,width,height afaict.)
Comment 8 Benjamin Smedberg [:bsmedberg] 2010-10-26 10:52:50 PDT
I have verified that Flash does in fact use the NPWindow.clipRect on Windows, because when I put in incorrect values, it only painted part of the screen.
Comment 9 Karl Tomlinson (:karlt) 2012-02-06 18:08:32 PST
This was fixed again in bug 571538
http://hg.mozilla.org/mozilla-central/rev/4a4c5a2cd08e

Note You need to log in before you can comment on or make changes to this bug.