[webkit-reviews] review granted: [Bug 7907] Replace more uses of DeprecatedString with String : [Attachment 7228] Remove a few more DeprecatedString uses while I'm at it...

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Wed Mar 22 08:24:37 PST 2006


Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 7907: Replace more uses of DeprecatedString with String
http://bugzilla.opendarwin.org/show_bug.cgi?id=7907

Attachment 7228: Remove a few more DeprecatedString uses while I'm at it...
http://bugzilla.opendarwin.org/attachment.cgi?id=7228&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+    if (s.isEmpty())
+	 return String(cs);

You don't need all these isEmpty checks. The String + operator already handles
them; an additional check doesn't add value.

The AccessibilityObjectCache changes are going to collide with my near-complete
rewrite of that file due to fixing it for Garbage Collection (and yes, I had
removed use of DeprecatedString).

I'm not sure about the "get values directly from settings" change. It's a nice
simplification. But it means that some settings that used to stay the same
until location change will now be "live". That means you could start loading a
page with JavaScript disabled and then later have it enabled or vice versa. I
believe that can lead to problems, although I'm not certain. Since you're
changing the semantics you will need to test.

r=me, assuming you test the settings change in Frame



More information about the webkit-reviews mailing list