Bug 73534

Created attachment 117370 [details] Patch

Please capitalise the first character of the bug title.

(In reply to comment #2) > Please capitalise the first character of the bug title. ok, will update...

Created attachment 117827 [details] Patch

Comment on attachment 117827 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117827&action=review > Source/WebCore/ChangeLog:18 > + Initial upstream, no new tests. > + > + > + * platform/blackberry/MIMETypeRegistryBlackBerry.cpp: Added. extra new line. > Source/WebCore/platform/blackberry/MIMETypeRegistryBlackBerry.cpp:135 > +bool MIMETypeRegistry::isApplicationPluginMIMEType(const String& mimeType) Either do bool MIMETypeRegistry::isApplicationPluginMIMEType(const String& /*mimeType*/) or add a UNUSED_PARAM(mimeType) line. > Source/WebCore/platform/blackberry/PlatformKeyboardEventBlackBerry.cpp:2 > + * Copyright (C) 2009, 2010 Research In Motion Limited. All rights reserved. 2011? > Source/WebCore/platform/blackberry/PlatformKeyboardEventBlackBerry.cpp:485 > +void PlatformKeyboardEvent::getCurrentModifierState(bool& shiftKey, bool& ctrlKey, bool& altKey, bool& metaKey) UNNEEDED_PARAM(xxx)

Created attachment 117836 [details] Patch

Comment on attachment 117836 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117836&action=review Informal review. > Source/WebCore/platform/blackberry/MIMETypeRegistryBlackBerry.cpp:107 > +String MIMETypeRegistry::getMIMETypeForExtension(const String &ext) > +{ > + String s = ext.lower(); > + ext--> extension? s--> lowerExtension? > Source/WebCore/platform/blackberry/MIMETypeRegistryBlackBerry.cpp:113 > + const ExtensionMap* e = extensionMap; > + while (e->extension) { > + if (s == e->extension) > + return e->mimeType; > + ++e; > + } e --> extensionMap? > Source/WebCore/platform/blackberry/MIMETypeRegistryBlackBerry.cpp:123 > + String s = type.lower(); s --> lowerMIMEType? > Source/WebCore/platform/blackberry/MIMETypeRegistryBlackBerry.cpp:130 > + const ExtensionMap* e = extensionMap; > + while (e->mimeType) { > + if (s == e->mimeType) > + return e->extension; > + ++e; > + } e --> extensionMap? > Source/WebCore/platform/blackberry/MIMETypeRegistryBlackBerry.cpp:136 > +bool MIMETypeRegistry::isApplicationPluginMIMEType(const String& /*mimeType*/) > +{ remove the comment > Source/WebCore/platform/blackberry/PlatformKeyboardEventBlackBerry.cpp:22 > +#include "BlackBerryPlatformLog.h" Use angle brackets for BlackBerryPlatformLog.h and move it down after #include <BlackBerryPlatformKeyboardEvent.h> > Source/WebCore/platform/blackberry/PlatformKeyboardEventBlackBerry.cpp:427 > +static inline WebCore::PlatformKeyboardEvent::Type toWebCorePlatformKeyboardEventType(const BlackBerry::Platform::KeyboardEvent::Type type) > +{ Is namespace WebCore necessary here? > Source/WebCore/platform/blackberry/PlatformKeyboardEventBlackBerry.cpp:435 > + default: > + return WebCore::PlatformKeyboardEvent::Char; Ditto. > Source/WebCore/platform/blackberry/PlatformKeyboardEventBlackBerry.cpp:457 > + m_type = toWebCorePlatformKeyboardEventType(event.type()); > + m_unmodifiedCharacter = event.character(); > + m_keyIdentifier = keyIdentifierForBlackBerryCharacter(event.character()); > + m_windowsVirtualKeyCode = windowsKeyCodeForBlackBerryCharacter(event.character()); > + unsigned short character = adjustCharacterFromOS(event.character()); > + m_text = String(&character, 1); > + m_unmodifiedText = m_text; > + > + if (event.character() == KEYCODE_BACK_TAB) > + m_shiftKey = true; // BackTab should be treated as Shift + Tab. > + Why can't all these data members be initialised in the initialisation list of the constructor?

accept most part, thanks for your comments, Leo

Created attachment 118348 [details] Patch

Comment on attachment 118348 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118348&action=review Almost done, but since this needs cq+ the nit needs to be fixed first. > Source/WebCore/platform/blackberry/PlatformKeyboardEventBlackBerry.cpp:482 > +void PlatformKeyboardEvent::getCurrentModifierState(bool& shiftKey, bool& ctrlKey, bool& altKey, bool& metaKey) The parameter names can be best removed or marked as unused.

Created attachment 118516 [details] Patch

Comment on attachment 118516 [details] Patch Looks good!

Comment on attachment 118516 [details] Patch Clearing flags on attachment: 118516 Committed r102426: <http://trac.webkit.org/changeset/102426>

All reviewed patches have been landed. Closing bug.