Bug 54584 - Summary: HTML5 <details> and <summary>: HTMLSummaryElement
Summary: Summary: HTML5 <details> and <summary>: HTMLSummaryElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Luiz Agostini
URL:
Keywords:
Depends on:
Blocks: 51071
  Show dependency treegraph
 
Reported: 2011-02-16 13:57 PST by Luiz Agostini
Modified: 2011-02-17 13:49 PST (History)
3 users (show)

See Also:


Attachments
patch (14.62 KB, patch)
2011-02-16 14:07 PST, Luiz Agostini
tkent: review-
Details | Formatted Diff | Diff
patch (14.63 KB, patch)
2011-02-17 08:07 PST, Luiz Agostini
no flags Details | Formatted Diff | Diff
patch (14.77 KB, patch)
2011-02-17 08:15 PST, Luiz Agostini
akling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luiz Agostini 2011-02-16 13:57:41 PST
Adding class HTMLSummaryElement to build systems.
Comment 1 Luiz Agostini 2011-02-16 14:07:12 PST
Created attachment 82690 [details]
patch
Comment 2 WebKit Review Bot 2011-02-16 14:09:20 PST
Attachment 82690 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1

Source/WebCore/html/HTMLSummaryElement.cpp:21:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/html/HTMLSummaryElement.cpp:22:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/html/HTMLSummaryElement.h:30:  The parameter name "document" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 WebKit Review Bot 2011-02-16 14:18:28 PST
Attachment 82690 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7927332
Comment 4 Kent Tamura 2011-02-17 06:38:13 PST
Comment on attachment 82690 [details]
patch

Why HTMLSummaryElement.{cpp,h} are needed?  ChangeLog should explain it.

Please fix style errors.
Comment 5 Luiz Agostini 2011-02-17 08:07:09 PST
Created attachment 82809 [details]
patch
Comment 6 Luiz Agostini 2011-02-17 08:15:50 PST
Created attachment 82811 [details]
patch
Comment 7 Luiz Agostini 2011-02-17 09:15:40 PST
Latest patch just adds some explanations to ChangeLog.
Comment 8 Luiz Agostini 2011-02-17 10:28:43 PST
rationale: https://bugs.webkit.org/show_bug.cgi?id=51071#c43
Comment 9 Luiz Agostini 2011-02-17 13:49:45 PST
Committed r78914: <http://trac.webkit.org/changeset/78914>