Series of patches to bring up to date and flesh out various aspects of the Haiku port.
Created attachment 48023 [details] Separate patch to fix small coding style violations. Patch can be applied individually.
Created attachment 48025 [details] This patch changes the MIMETypeRegistryHaiku implementation to fall back to the Haiku MIME database for unknown file extensions. Patch can be applied individually.
Created attachment 48027 [details] Implements homeDirectoryPath() and listDirectory() on the Haiku platform. Can be applied individually.
Created attachment 48031 [details] Fixes to rect conversion and image rendering on Haiku The patch can be applied individually. The changes to the rect conversions are indeed correct. In Haiku (to stay compatibly with BeOS), a BRect specifies the left/top and bottom/right pixel *indices*, even though the values are floating point. So a rectangle covering just one pixel would be specified as BRect(0, 0, 0, 0). In WebCore and other frame works, such rectangles would be expressed as 0, 0, 1, 1. In WebCore, the width and height of rectangles refer to the distance between pixels, while on Haiku, a one pixel rect has indeed a width and height of 0, as confusing as that may be. The part of the patch that affects WebCore/platform/graphics/haiku/ImageHaiku.cpp also implements the drawing methods more correctly. Image observers are notified, and pattern drawing takes the "phase" into account which makes scrolled backgrounds render correctly. Transformations are still not supported, since the Haiku drawing backend itself does not yet support them.
Created attachment 48032 [details] Implementing PathHaiku and GradientHaiku, fleshing out GraphicsContextHaiku The patch can be applied individually, but cannot be broken into smaller pieces. PathHaiku was irritatingly implemented as BRegion, while BRegion has nothing to do with paths. BShape is the way to go. I've implemented the subset of functionality that can be implemented using BShape. PathGradient was previously not implemented at all. I've taken the Qt implementation as a reference and websites using gradients seem to render fine. GraphicsContextHaiku was not implementing a great deal of functionality. The patch implements * partial support for paths and gradients * more clipping methods * support for round rects * support for transparency layers * support for translating the context, as needed for scrolling * changes the strokeArc() method to produce beautiful arcs as needed for box elements without the need to implement clipping paths. * implements filling rects * implements rounding to device pixels * a few missing new methods that broke the build on Haiku * fixes a few warnings
Attachment 48032 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:63: Missing space before { [whitespace/braces] [5] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:64: Missing space before { [whitespace/braces] [5] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:75: Missing space before { [whitespace/braces] [5] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:76: Missing space before { [whitespace/braces] [5] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:312: Should have a space between // and comment [whitespace/comments] [4] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:325: One line control clauses should not use braces. [whitespace/braces] [4] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:423: Should have a space between // and comment [whitespace/comments] [4] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:432: Should have a space between // and comment [whitespace/comments] [4] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:436: One line control clauses should not use braces. [whitespace/braces] [4] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:450: One line control clauses should not use braces. [whitespace/braces] [4] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:452: One line control clauses should not use braces. [whitespace/braces] [4] WebCore/platform/graphics/haiku/GradientHaiku.cpp:54: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 12 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 48033 [details] Implementing PathHaiku and GradientHaiku, fleshing out GraphicsContextHaiku The attachment 48032 [details] had all the explanation, don't know how to retrieve it. This version fixes some style issues pointed out by the style-checker-bot.
Attachment 48033 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:63: Missing space before { [whitespace/braces] [5] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:64: Missing space before { [whitespace/braces] [5] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:75: Missing space before { [whitespace/braces] [5] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:76: Missing space before { [whitespace/braces] [5] Total errors found: 4 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 48036 [details] Various fixes to mouse/wheel/keyboard event propagation on Haiku This patch can be applied individually. * Some compiler warnings were fixed. * Wrong coordinates were extracted from the Haiku event BMessage, resulting in an offset for mouse moved events. * Improved the compatibility of keyboard events. * Use the correct code for the tab/backspace key in EventHandlerHaiku.cpp and fix the build by not including PlatformScrollBar.h * Generate a correct timestamp for mouse/wheel events. The value is documented as being in seconds. * Fill the modifier flags in mouse/wheel events.
All the patches are against r54275.
Attachment 48036 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/haiku/PlatformWheelEventHaiku.cpp:56: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/platform/haiku/PlatformWheelEventHaiku.cpp:57: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/platform/haiku/PlatformWheelEventHaiku.cpp:58: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/platform/haiku/PlatformWheelEventHaiku.cpp:59: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/platform/haiku/PlatformMouseEventHaiku.cpp:78: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/platform/haiku/PlatformMouseEventHaiku.cpp:79: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/platform/haiku/PlatformMouseEventHaiku.cpp:80: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/platform/haiku/PlatformMouseEventHaiku.cpp:81: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 8 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 48037 [details] HaikuLauncher minimal browser shell This patch adds an implementation for the Haiku browser shell. There was no previous implementation checked into the repository, except for the WebCoreSupport classes. There are minimal changes there, but the important detail of the HaikuLauncher code is that it uses an offscreen bitmap/view combination to perform drawing, and that any events which happen in the window thread are forwarded to a new class WebProcess, which is an event handler in the main thread. Thus, all calls into WebKit happen on the main thread. Except for keyboard input, the HaikuLauncher is functional. Some sites result in "about:blank" to be loaded halfway through loading the site (but without actually showing an about:blank page). After such an event, the shell becomes unusable and will soon crash when calling some pure virtual method.
Created attachment 48038 [details] Various fixes to mouse/wheel/keyboard event propagation on Haiku Fixed style violations of previous patch.
Created attachment 48039 [details] Implementing PathHaiku and GradientHaiku, fleshing out GraphicsContextHaiku Fixed remaining coding style issues, though I don't know if the style script is correct in this case.
Attachment 48039 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:69: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:70: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:71: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:72: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:74: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:75: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:76: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:77: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:88: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:89: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:90: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:91: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:93: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:94: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:95: Tab found; better to use spaces [whitespace/tab] [1] WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:96: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 16 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 48040 [details] Implementing PathHaiku and GradientHaiku, fleshing out GraphicsContextHaiku Darn, forgot to fix up tabs. Sorry about the mess! Should pass the style check now.
Comment on attachment 48023 [details] Separate patch to fix small coding style violations. Whoops. This need a ChangeLog.
Comment on attachment 48025 [details] This patch changes the MIMETypeRegistryHaiku implementation to fall back to the Haiku MIME database for unknown file extensions. All of these patches need changelogs. See http://webkit.org/coding/contributing.html (prepare-ChangeLog is the command). Please look at other entries in the ChangeLog to note typical formatting. Here's some details on ChangeLog formatting: Leave the Reviewed by NOBODY (OOPS) but remove the no tests oops and either say what tests cover the change or why there are no tests added. Other typical formatting is Bug Title Bug Link Small description or even better function by function small descriptions of the changes done (see other changelog entries for good examples).
Comment on attachment 48027 [details] Implements homeDirectoryPath() and listDirectory() on the Haiku platform. No ChangeLog.
Comment on attachment 48031 [details] Fixes to rect conversion and image rendering on Haiku No ChangeLog.
Comment on attachment 48037 [details] HaikuLauncher minimal browser shell No ChangeLog.
Comment on attachment 48038 [details] Various fixes to mouse/wheel/keyboard event propagation on Haiku No ChangeLog.
Comment on attachment 48040 [details] Implementing PathHaiku and GradientHaiku, fleshing out GraphicsContextHaiku No ChangeLog.
Thanks a lot for the comment. I was aware that the Changelogs are missing, but was confused how those are attached. Are they supposed to be part of the patch as if I modified the Changelog file? I've read through the contributor documentation, but I didn't know what to do about the problem that I need to separate all my changes into smaller patches. I figured I can't use those scripts then. I'll figure it out and replace the patches.
(In reply to comment #24) > Thanks a lot for the comment. I was aware that the Changelogs are missing, but > was confused how those are attached. Are they supposed to be part of the patch > as if I modified the Changelog file? Yes. >I didn't know what to do about the problem that I need to > separate all my changes into smaller patches. If I understand what you're asking, you're saying that yo have all of these patches in your tree at once so the ChangeLog would be clutter with too many things. Well, some people use svn-create-patch and save a diff. Then create little patches for each change that they apply/unapply. Other people use git. I guess another approach would be to create the ChangeLog and edit it separately for each patch (but admittedly that sounds painful). It is hard to say what would work best for you.
Created attachment 48105 [details] Implementing PathHaiku and GradientHaiku, fleshing out GraphicsContextHaiku Now with changelog.
Created attachment 48108 [details] Various fixes to mouse/wheel/keyboard event propagation on Haiku Now with changelog.
Created attachment 48111 [details] Implementing PathHaiku and GradientHaiku, fleshing out GraphicsContextHaiku Really sorry for the mess. This time I've used a different method of creating the patch, so hopefully it applies cleanly now.
Created attachment 48114 [details] Various fixes to mouse/wheel/keyboard event propagation on Haiku Ok, reassured by the successful style check, I am using the same method to create a patch for the rest of the patches with added ChangeLog diff.
Created attachment 48116 [details] Fixes to rect conversion and image rendering on Haiku Patch includes ChangeLog now.
Created attachment 48117 [details] Implements homeDirectoryPath() and listDirectory() on the Haiku platform. Patch now includes ChangeLog.
Created attachment 48119 [details] Separate patch to fix small coding style violations. Patch now includes diff to ChangeLog.
Created attachment 48120 [details] This patch changes the MIMETypeRegistryHaiku implementation to fall back to the Haiku MIME database for unknown file extensions. Patch includes diff to ChangeLog now.
Created attachment 48122 [details] HaikuLauncher minimal browser shell Patch now contains a ChangeLog. CheckerBoardView has been removed, it was added accidentally.
Attachment 48122 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 Last 3072 characters of output: e use: LeakTracker_h [build/header_guard] [5] WebKit/haiku/HaikuLauncher/LeakTracker.h:34: One line control clauses should not use braces. [whitespace/braces] [4] WebKit/haiku/HaikuLauncher/LeakTracker.h:52: This { should be at the end of the previous line [whitespace/braces] [4] WebKit/haiku/HaikuLauncher/LeakTracker.h:53: Missing space before ( in if( [whitespace/parens] [5] WebKit/haiku/HaikuLauncher/LeakTracker.h:54: This { should be at the end of the previous line [whitespace/braces] [4] WebKit/haiku/HaikuLauncher/LeakTracker.h:71: Alphabetical sorting problem. [build/include_order] [4] WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp:133: One line control clauses should not use braces. [whitespace/braces] [4] WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp:292: Should have a space between // and comment [whitespace/comments] [4] WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp:293: Should have a space between // and comment [whitespace/comments] [4] WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp:294: Should have a space between // and comment [whitespace/comments] [4] WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp:295: Should have a space between // and comment [whitespace/comments] [4] WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp:315: Should have a space between // and comment [whitespace/comments] [4] WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp:321: Should have a space between // and comment [whitespace/comments] [4] WebKit/haiku/WebCoreSupport/EditCommandHaiku.h:32: #ifndef header guard has wrong style, please use: EditCommandHaiku_h [build/header_guard] [5] WebKit/haiku/API/WebView.cpp:39: Alphabetical sorting problem. [build/include_order] [4] WebKit/haiku/API/WebView.cpp:165: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/haiku/API/WebView.cpp:166: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/haiku/API/WebView.cpp:167: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/haiku/HaikuLauncher/LauncherApp.h:27: #ifndef header guard has wrong style, please use: LauncherApp_h [build/header_guard] [5] WebKit/haiku/HaikuLauncher/LauncherApp.cpp:49: LOAD_AT_STARTING is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] WebKit/haiku/HaikuLauncher/LauncherApp.cpp:71: Use 0 instead of NULL. [readability/null] [5] WebKit/haiku/API/WebFrame.h:42: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit/haiku/WebCoreSupport/ChromeClientHaiku.h:36: Alphabetical sorting problem. [build/include_order] [4] WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp:595: Missing space after , [whitespace/comma] [3] WebKit/haiku/API/WebProcess.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit/haiku/API/WebProcess.h:70: More than one command on the same line [whitespace/newline] [4] Total errors found: 39 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 48123 [details] Implementation of FontPlatformData for Haiku port. This file was implemented by Maxime Simone. I have no idea why it was not added to the repository yet, but it is certainly needed by the Haiku port. Maxime is CC on this ticket, so maybe he can comment. To me, the code looks fine as a first implementation. Eventually, it should use more of Haiku's font API to remove some hard coded font names, but for now it will definitely work fine.
Created attachment 48124 [details] More robust conversion from BString to String. Another patch by Maxime Simone. It could be that this more robust conversion is not needed anymore since some recent changes to Haiku's BString implementation. However, since the Haiku API does not throw on allocation failures, checking BString::String() against NULL is more correct indeed.
Attachment 48124 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/text/haiku/StringHaiku.cpp:29: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 48125 [details] Implementation of GlyphPage::fill() for Haiku port. Patch by Ryan Leavengood (I assume based on copyright). The file is necessary for the Haiku port to work, but I don't know myself what it actually is. Maybe Maxime can comment.
(In reply to comment #34) > Created an attachment (id=48122) [details] > HaikuLauncher minimal browser shell > > Patch now contains a ChangeLog. CheckerBoardView has been removed, it was added > accidentally. I think you should create a separated bug for this part of the port. Having multiple tiny bugs (with a patch per bug) is easier to review, and also your patches would be handled faster.
(In reply to comment #36) > Created an attachment (id=48123) [details] > Implementation of FontPlatformData for Haiku port. > > This file was implemented by Maxime Simone. I have no idea why it was not added > to the repository yet, but it is certainly needed by the Haiku port. Maxime is > CC on this ticket, so maybe he can comment. To me, the code looks fine as a > first implementation. Eventually, it should use more of Haiku's font API to > remove some hard coded font names, but for now it will definitely work fine. There was a bug filled in bugzilla about these files. You may find it. In fact it was discussed but I thought not including it it for the moment to be better. Maybe someone more aware of this part of WebKit can comment.
(In reply to comment #39) > Created an attachment (id=48125) [details] > Implementation of GlyphPage::fill() for Haiku port. > > Patch by Ryan Leavengood (I assume based on copyright). The file is necessary > for the Haiku port to work, but I don't know myself what it actually is. Maybe > Maxime can comment. Ditoo previous comment (this was discussed but not commited). Would need comments from the experts.
Comment on attachment 48122 [details] HaikuLauncher minimal browser shell Will create separate bug for the HaikuLauncher code.
The bug which tracked comments for FontPlatformData is https://bugs.webkit.org/show_bug.cgi?id=28131. I agree with Eric's review and will try to address the remaining issues.
(In reply to comment #44) > The bug which tracked comments for FontPlatformData is > https://bugs.webkit.org/show_bug.cgi?id=28131. I agree with Eric's review and > will try to address the remaining issues. I was searching it as well (but you were quicker). Indeed this should really be improved.
Comment on attachment 48119 [details] Separate patch to fix small coding style violations. Clearing flags on attachment: 48119 Committed r54383: <http://trac.webkit.org/changeset/54383>
Comment on attachment 48124 [details] More robust conversion from BString to String. Clearing flags on attachment: 48124 Committed r54434: <http://trac.webkit.org/changeset/54434>
Comment on attachment 48125 [details] Implementation of GlyphPage::fill() for Haiku port. Clearing flags on attachment: 48125 Committed r54442: <http://trac.webkit.org/changeset/54442>
Can we retire this epic bug and use one patch per bug? "Improve/maintain Haiku port" is not a task that will ever be finished.
Comment on attachment 48111 [details] Implementing PathHaiku and GradientHaiku, fleshing out GraphicsContextHaiku I believe that this has been submitted in a separate bug which is great! (So I'm obsoleting this patch and removing r? to remove it from the review queue.)
Comment on attachment 48114 [details] Various fixes to mouse/wheel/keyboard event propagation on Haiku I believe that this has been submitted in a separate bug which is great! (So I'm obsoleting this patch and removing r? to remove it from the review queue.)
Comment on attachment 48116 [details] Fixes to rect conversion and image rendering on Haiku I believe that this has been submitted in a separate bug which is great! (So I'm obsoleting this patch and removing r? to remove it from the review queue.)
Comment on attachment 48117 [details] Implements homeDirectoryPath() and listDirectory() on the Haiku platform. I believe that this has been submitted in a separate bug which is great! (So I'm obsoleting this patch and removing r? to remove it from the review queue.)
Comment on attachment 48123 [details] Implementation of FontPlatformData for Haiku port. I believe that this has been submitted in a separate bug which is great! (So I'm obsoleting this patch and removing r? to remove it from the review queue.)
Comment on attachment 48120 [details] This patch changes the MIMETypeRegistryHaiku implementation to fall back to the Haiku MIME database for unknown file extensions. I believe that this has been submitted in a separate bug which is great! (So I'm obsoleting this patch and removing r? to remove it from the review queue.)
Thanks! Looks like there aren't any more patches here and its safe to retire this bug.