Bug 22324 - Add basic subset of WML elements
Summary: Add basic subset of WML elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks: 20393
  Show dependency treegraph
 
Reported: 2008-11-17 14:09 PST by Nikolas Zimmermann
Modified: 2008-11-17 16:27 PST (History)
0 users

See Also:


Attachments
1) Add WML tests (LayoutTests/fast/wml & WebCore/manual-tests/wml) (57.83 KB, patch)
2008-11-17 14:16 PST, Nikolas Zimmermann
staikos: review+
Details | Formatted Diff | Diff
2) Add WML build support (WebKitTools/Scripts/ changes) (5.31 KB, patch)
2008-11-17 14:17 PST, Nikolas Zimmermann
staikos: review+
Details | Formatted Diff | Diff
3) Add basic WML support & WebCore integration (91.28 KB, patch)
2008-11-17 14:17 PST, Nikolas Zimmermann
staikos: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2008-11-17 14:09:13 PST
The WML support should be added in a few reviewable chunks.
Based on the patches living in bug 20393, a new simpler patch will be created only adding a few WML elements (<card> / <p> / <a>) - but with a new design. WML elements don't inherit from HTML* elements anymore, CSS integratin is much simpler, no need for a new WMLTokenizer class etc.

Stay tuned, uploading patches soon.
Comment 1 Nikolas Zimmermann 2008-11-17 14:16:38 PST
Created attachment 25226 [details]
1) Add WML tests (LayoutTests/fast/wml & WebCore/manual-tests/wml)
Comment 2 Nikolas Zimmermann 2008-11-17 14:17:18 PST
Created attachment 25227 [details]
2) Add WML build support (WebKitTools/Scripts/ changes)
Comment 3 Nikolas Zimmermann 2008-11-17 14:17:56 PST
Created attachment 25228 [details]
3) Add basic WML support & WebCore integration
Comment 4 Nikolas Zimmermann 2008-11-17 16:05:20 PST
Landed in r38541.
Comment 5 Mark Rowe (bdash) 2008-11-17 16:20:53 PST
Comment on attachment 25228 [details]
3) Add basic WML support & WebCore integration

The .pro changes read as though WML will be enabled unless explicitly disabled.  Given that this is primarily intended for mobile devices, does it make sense for this to be the default behaviour?

A bunch of this code uses the incorrect "Wml" capitalization in method and variable names.

The use of ENABLE to wrap code for a specific library is not consistent with existing patterns: USE should be used instead.

Putting code wrapped in ENABLE(WBXML) directly in the cross-platform MainResourceLoader.cpp is not desirable.  The code should be refactored so that this isn't necessary.

The Torch Mobile copyright line appears to have two different forms. Is that intentional?
Comment 6 Nikolas Zimmermann 2008-11-17 16:27:01 PST
(In reply to comment #5)
> (From update of attachment 25228 [details] [review])
> The .pro changes read as though WML will be enabled unless explicitly disabled.
>  Given that this is primarily intended for mobile devices, does it make sense
> for this to be the default behaviour?
Just noticed and fixed - just an oversight.
 
> A bunch of this code uses the incorrect "Wml" capitalization in method and
> variable names.
I wasn't aware of this requirement. Will fix.

> The use of ENABLE to wrap code for a specific library is not consistent with
> existing patterns: USE should be used instead.
> 
> Putting code wrapped in ENABLE(WBXML) directly in the cross-platform
> MainResourceLoader.cpp is not desirable.  The code should be refactored so that
> this isn't necessary.
Okay, I'll join IRC to discuss :-)
 
> The Torch Mobile copyright line appears to have two different forms. Is that
> intentional?
Not sure, will have to pass that question to George.