Bug 73534

Summary:

upstream BlackBerry porting of MIMETypeRegistry/KeyboardEvent

Product:

WebKit

Reporter:

Mary Wu <mawu>

Component:

Platform

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dbates, leo.yang, staikos, tonikitoo, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

73144

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Mary Wu

Reported 2011-11-30 23:58:44 PST

add two files MIMETypeRegistryBlackBerry.cpp and PlatformKeyboardEventBlackBerry.cpp to WebCore/platform/blackberry

Attachments
Patch (18.84 KB, patch) 2011-12-01 00:35 PST, Mary Wu	no flags	Details Formatted Diff Diff
Patch (19.13 KB, patch) 2011-12-04 20:08 PST, Mary Wu	no flags	Details Formatted Diff Diff
Patch (19.16 KB, patch) 2011-12-04 21:14 PST, Mary Wu	no flags	Details Formatted Diff Diff
Patch (19.10 KB, patch) 2011-12-08 02:01 PST, Mary Wu	no flags	Details Formatted Diff Diff
Patch (19.07 KB, patch) 2011-12-08 19:16 PST, Mary Wu	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Mary Wu

Comment 1 2011-12-01 00:35:56 PST

Created attachment 117370 [details] Patch

Leo Yang

Comment 2 2011-12-01 02:03:57 PST

Please capitalise the first character of the bug title.

Mary Wu

Comment 3 2011-12-04 20:02:10 PST

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

Mary Wu

Comment 4 2011-12-04 20:08:55 PST

Created attachment 117827 [details] Patch

Antonio Gomes

Comment 5 2011-12-04 20:38:26 PST

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)

Mary Wu

Comment 6 2011-12-04 21:14:17 PST

Created attachment 117836 [details] Patch

Leo Yang

Comment 7 2011-12-07 22:10:24 PST

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?

Mary Wu

Comment 8 2011-12-08 01:50:32 PST

accept most part, thanks for your comments, Leo

Mary Wu

Comment 9 2011-12-08 02:01:17 PST

Created attachment 118348 [details] Patch

Rob Buis

Comment 10 2011-12-08 07:15:28 PST

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.

Mary Wu

Comment 11 2011-12-08 19:16:52 PST

Created attachment 118516 [details] Patch

Rob Buis

Comment 12 2011-12-08 19:35:57 PST

Comment on attachment 118516 [details] Patch Looks good!

WebKit Review Bot

Comment 13 2011-12-08 20:37:56 PST

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

WebKit Review Bot

Comment 14 2011-12-08 20:38:01 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.