Bug 14411

Summary: Regression: WebNetscapePluginPackage overagressively sets CurApRefNum, causes BBEdit to malfunction
Product: WebKit Reporter: Jim Correia <jim.correia>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Major CC: jim.correia, mrowe
Priority: P2 Keywords: InRadar
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Proposed patch for bug.
darin: review+
Revised patch. Updated ChangeLog comment, removed tabs. Removed cast to (void) in (+[WebNetscapePluginPackage initialize]). darin: review+

Description Jim Correia 2007-06-26 07:35:34 PDT
+[WebNetscapePluginPackage initialize], if invoked early enough in application startup, can result in CurApRefNum pointing at the system resource file instead of the application's bundle resource map.

The attached patch addresses this issue.

This affects all shipping versions of BBEdit, which rely on CurApRefNum pointing at the application's bundle resource map.

This is a new problem/regression in WebKit (vs. 10.4.x shipped versions) because the startup dependency order appears to have changed. (Creating an HIWebView/WebView results in the plug-in database getting initialized in trunk versions, which invokes the problematic code. This is a problem because, for historic reasons, BBEdit creates an HIWebView very early in the startup process, before the Resource Manager has been lazy initialized and has opened the resource map.)
Comment 1 Jim Correia 2007-06-26 07:37:54 PDT
Created attachment 15246 [details]
Proposed patch for bug.
Comment 2 Darin Adler 2007-06-26 13:43:15 PDT
Comment on attachment 15246 [details]
Proposed patch for bug.

Good change.

Tab in the ChangeLog will need ot be removed to check in. Also I'd like the comment to mention the name and brief description of the bug, not just the ID.

Also, the (void) cast, while a common way to indicate that a function value is ignored, is not something we generally do in WebKit/Core code. I'd prefer to leave it out unless we have a tool that complains about ignored return values. I think the emphasis on the fact that the function is being used for its side effect is a good thing, but I think the comment already makes that clear and the ugly syntax is only really worthwhile in code where it's consistently used.
Comment 3 Jim Correia 2007-06-26 13:51:09 PDT
Created attachment 15259 [details]
Revised patch. Updated ChangeLog comment, removed tabs. Removed cast to (void) in (+[WebNetscapePluginPackage initialize]).
Comment 4 Jim Correia 2007-06-26 13:53:13 PDT
Comment on attachment 15259 [details]
Revised patch. Updated ChangeLog comment, removed tabs. Removed cast to (void) in (+[WebNetscapePluginPackage initialize]).

Updated ChangeLog comment, removed tabs. Removed cast to (void) in (+[WebNetscapePluginPackage initialize]).
Comment 5 Darin Adler 2007-06-26 13:54:13 PDT
Comment on attachment 15259 [details]
Revised patch. Updated ChangeLog comment, removed tabs. Removed cast to (void) in (+[WebNetscapePluginPackage initialize]).

r=me
Comment 6 Mark Rowe (bdash) 2007-06-26 20:13:08 PDT
<rdar://problem/5297268>
Comment 7 Mark Rowe (bdash) 2007-06-26 20:14:22 PDT
Landed in r23805.  Thanks for the patch!