Closed
Bug 469830
Opened 16 years ago
Closed 15 years ago
need Windows drawing code for test plugin
Categories
(Core Graveyard :: Plug-ins, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jaas, Assigned: jimm)
References
Details
Attachments
(3 files, 2 obsolete files)
382 bytes,
text/html
|
Details | |
393 bytes,
text/html
|
Details | |
5.46 KB,
patch
|
Details | Diff | Splinter Review |
We need some Windows drawing code for the test plugin. The Mac OS X implementation already has drawing code. Relevant files: modules/plugin/test/testplugin/nptest_platform.h modules/plugin/test/testplugin/nptest_windows.cpp modules/plugin/test/testplugin/nptest_macosx.mm
Assignee | ||
Updated•16 years ago
|
Priority: -- → P3
Comment 1•16 years ago
|
||
If you do this, please track the changes in bug 473577 so we can use the plugin for reftests. (They're pretty simple.)
Assignee | ||
Comment 2•16 years ago
|
||
Assignee | ||
Updated•16 years ago
|
Attachment #359079 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 3•16 years ago
|
||
Comment on attachment 359079 [details] [diff] [review] windows test plugin rendering v.1 Ted would you be a good reviewer for this?
Assignee | ||
Comment 4•16 years ago
|
||
I just noticed a litle left over debugging code in this in the use of pluginDrawWin instead of pluginDraw. I'll clean that up before this lands.
Comment 5•16 years ago
|
||
You should probably have Josh give it a once-over (since he's the module owner), but I'll take a look as well. One thing I noticed that you didn't handle was the alpha value (which I conveniently forgot to write a reftest for). We should support drawing with partial transparency. I'll attach a testcase (that I should turn into a reftest) in a sec.
Comment 6•16 years ago
|
||
Comment 7•16 years ago
|
||
This and the previous test attachment should render identically if the test plugin supports the alpha channel properly.
Assignee | ||
Comment 8•16 years ago
|
||
To get transparency support I had to switch from the ancient GDI to the newer GDI+. As an example plug-in that's actually better. However the plugin will fail to load on older operating systems that don't have GDI+ available. XP came with it out of the box so I don't believe this will cause any problems for tinderbox.
Attachment #359079 -
Attachment is obsolete: true
Attachment #359079 -
Flags: review?(ted.mielczarek)
Assignee | ||
Updated•16 years ago
|
Attachment #359335 -
Flags: superreview?(joshmoz)
Attachment #359335 -
Flags: review?(ted.mielczarek)
Updated•16 years ago
|
Attachment #359335 -
Flags: review?(ted.mielczarek) → review+
Comment 9•16 years ago
|
||
Comment on attachment 359335 [details] [diff] [review] windows test plugin rendering v.2 + if (pe == NULL || instanceData == NULL || + instanceData->window.type != NPWindowTypeDrawable) return 0; Personally I would put that return on the next line. +static void +GetColorsFromRGBA(PRUint32 rgba, BYTE* r, BYTE* g, BYTE* b, BYTE* a) You could make this utility method just return a Color or a SolidBrush, but it's not a big deal either way. Otherwise looks fine to me. I think using GDI+ should be fine, since it's only for tests (and for reference) anyway.
Reporter | ||
Comment 10•16 years ago
|
||
Comment on attachment 359335 [details] [diff] [review] windows test plugin rendering v.2 What Ted said :)
Attachment #359335 -
Flags: superreview?(joshmoz) → review+
Comment 11•16 years ago
|
||
Note that we'll have to modify the reftest conditions here as well before checking this in: http://mxr.mozilla.org/mozilla-central/source/modules/plugin/test/reftest/reftest.list
Assignee | ||
Comment 12•16 years ago
|
||
Assignee | ||
Updated•16 years ago
|
Attachment #359335 -
Attachment is obsolete: true
Comment 13•15 years ago
|
||
Would you like me to push this for you?
Assignee | ||
Comment 14•15 years ago
|
||
(In reply to comment #13) > Would you like me to push this for you? I'll get it, I was planning on waiting till the weekend.
Assignee | ||
Comment 15•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/f46ab3393b32
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 16•15 years ago
|
||
Comment on attachment 359563 [details] [diff] [review] windows test plugin rendering v.3 >-random-if(!haveTestPlugin||MOZ_WIDGET_TOOLKIT!="cocoa") != plugin-sanity.html about:blank >-fails-if(!haveTestPlugin||MOZ_WIDGET_TOOLKIT!="cocoa") == plugin-sanity.html div-sanity.html >+random-if(!haveTestPlugin||MOZ_WIDGET_TOOLKIT!="cocoa"||MOZ_WIDGET_TOOLKIT!="windows") != plugin-sanity.html about:blank >+fails-if(!haveTestPlugin||MOZ_WIDGET_TOOLKIT!="cocoa"||MOZ_WIDGET_TOOLKIT!="windows") == plugin-sanity.html div-sanity.html Shouldn't this rather be +random-if(!haveTestPlugin||(MOZ_WIDGET_TOOLKIT!="cocoa"&&MOZ_WIDGET_TOOLKIT!="windows")) != plugin-sanity.html about:blank +fails-if(!haveTestPlugin||(MOZ_WIDGET_TOOLKIT!="cocoa"&&MOZ_WIDGET_TOOLKIT!="windows")) == plugin-sanity.html div-sanity.html ?
Assignee | ||
Comment 17•15 years ago
|
||
(In reply to comment #16) > (From update of attachment 359563 [details] [diff] [review]) > >-random-if(!haveTestPlugin||MOZ_WIDGET_TOOLKIT!="cocoa") != plugin-sanity.html about:blank > >-fails-if(!haveTestPlugin||MOZ_WIDGET_TOOLKIT!="cocoa") == plugin-sanity.html div-sanity.html > >+random-if(!haveTestPlugin||MOZ_WIDGET_TOOLKIT!="cocoa"||MOZ_WIDGET_TOOLKIT!="windows") != plugin-sanity.html about:blank > >+fails-if(!haveTestPlugin||MOZ_WIDGET_TOOLKIT!="cocoa"||MOZ_WIDGET_TOOLKIT!="windows") == plugin-sanity.html div-sanity.html > > Shouldn't this rather be > > +random-if(!haveTestPlugin||(MOZ_WIDGET_TOOLKIT!="cocoa"&&MOZ_WIDGET_TOOLKIT!="windows")) > != plugin-sanity.html about:blank > +fails-if(!haveTestPlugin||(MOZ_WIDGET_TOOLKIT!="cocoa"&&MOZ_WIDGET_TOOLKIT!="windows")) > == plugin-sanity.html div-sanity.html > > ? I'll update it. Thx.
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•