| Summary: | Remove older versions of custom Carbon events in npapi.h | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Stuart Morgan <stuartmorgan> | ||||||
| Component: | Plug-ins | Assignee: | Stuart Morgan <stuartmorgan> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, andersca, commit-queue, eric, joshmoz, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
Created attachment 59040 [details]
fix v1
Note that the test plugin is still using the old names; the value of having them match seemed minimal, and this means test expectations don't have to all change.
Comment on attachment 59040 [details]
fix v1
Seems great to fix everything in our own code. But why are we removing these macros from a public header? Are you sure this is OK? Did Netscape already do this in their copy of npapi.h?
Setting review+ assuming the answer to all those questions is "a good reason", yes, and yes.
The answers are: - To bring WebKit's copy of the header more in line with http://code.google.com/p/npapi-headers/ in order to minimize confusion both for people writing plugins ("which header should I build my plugin against?") and for browser developers (when updating the headers for NPAPI changes). - People building plugins against the WebKit version of the header will have to make minor code changes when building against the new header, but that will be an easy change; I think the benefits outweigh the cost. (And hopefully plugin vendors will start using the central repository instead of an individual browser's copy anyway.) - Mozilla already did this. Created attachment 65928 [details]
fix, unbitrotted
Same patch, but with bitrot from the move of TestNetscapePlugIn's main.cpp addressed.
Comment on attachment 59040 [details] fix v1 Cleared Darin Adler's review+ from obsolete attachment 59040 [details] so that this bug does not appear in http://webkit.org/pending-commit. Could you also set commit-queue+ then, since it is in fact a pending commit? It would be nice not to have to un-bit-rot this a second time. Sigh; these defines have now spread to WebKitTools/DumpRenderTree/chromium/TestNetscapePlugIn/ForwardingHeaders/WebKit/npfunctions.h Can someone explain why this has been commit-queue? for almost two months? If there's something wrong with the patch I'd be happy to fix it. Comment on attachment 65928 [details]
fix, unbitrotted
OK.
The commit-queue encountered the following flaky tests while processing attachment 65928 [details]:
fast/workers/storage/use-same-database-in-page-and-workers.html
Please file bugs against the tests. The commit-queue is continuing to process your patch.
Comment on attachment 65928 [details] fix, unbitrotted Clearing flags on attachment: 65928 Committed r69979: <http://trac.webkit.org/changeset/69979> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/69979 might have broken Leopard Intel Debug (Tests) |
WebKit's npapi.h defines three events for Carbon plugin event structures: #define getFocusEvent (osEvt + 16) #define loseFocusEvent (osEvt + 17) #define adjustCursorEvent (osEvt + 18) But the upstream copy uses an enum for those values: enum NPEventType { NPEventType_GetFocusEvent = (osEvt + 16), NPEventType_LoseFocusEvent, NPEventType_AdjustCursorEvent, ... Use of the former should be removed from WebKit, and the #defines removed from the header.