[Bug 38140] Initial support for HTMLMeterElement
0 views
Skip to first unread message
bugzill...@webkit.org
May 12, 2010, 12:33:58 PM5/12/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
--- Comment #21 from Kent Tamura <tk...@chromium.org> 2010-05-12 09:33:56 PST ---
(From update of attachment 55016)
WebCore/html/HTMLMeterElement.h:54
+ virtual bool isOptionalFormControl() const { return true; }
This seems curious. Why do we need to return false for isOptionalFormControl()?
Could you add a test confirming that <meter> tag behavior is not changed on platforms without ENABLE_METER_TAG?
For example, "<meter>56%</meter>" should work like "<span>56%</span>"
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
--- Comment #21 from Kent Tamura <tk...@chromium.org> 2010-05-12 09:33:56 PST ---
(From update of attachment 55016)
WebCore/html/HTMLMeterElement.h:54
+ virtual bool isOptionalFormControl() const { return true; }
This seems curious. Why do we need to return false for isOptionalFormControl()?
Could you add a test confirming that <meter> tag behavior is not changed on platforms without ENABLE_METER_TAG?
For example, "<meter>56%</meter>" should work like "<span>56%</span>"
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
bugzill...@webkit.org
May 12, 2010, 1:04:59 PM5/12/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
--- Comment #22 from Yael <yael....@nokia.com> 2010-05-12 10:04:57 PST ---
(In reply to comment #21)
> (From update of attachment 55016 [details])
> Could you add a test confirming that <meter> tag behavior is not changed on platforms without ENABLE_METER_TAG?
> For example, "<meter>56%</meter>" should work like "<span>56%</span>"
This was discussed before, in https://bugs.webkit.org/show_bug.cgi?id=35937. There is no mechanism for adding conditions to html.css, so the new style definition will affect ports with this flag turned off. There is precedence with other new tags e.g. datagrid for doing the same.
--- Comment #22 from Yael <yael....@nokia.com> 2010-05-12 10:04:57 PST ---
(In reply to comment #21)
> (From update of attachment 55016 [details])
> WebCore/html/HTMLMeterElement.h:54
> + virtual bool isOptionalFormControl() const { return true; }
> This seems curious. Why do we need to return false for isOptionalFormControl()?
>
From http://dev.w3.org/html5/spec/Overview.html#attr-input-required , the required attribute applies only to input (and textarea) elements. All other form control elements return true.
> + virtual bool isOptionalFormControl() const { return true; }
> This seems curious. Why do we need to return false for isOptionalFormControl()?
>
> Could you add a test confirming that <meter> tag behavior is not changed on platforms without ENABLE_METER_TAG?
> For example, "<meter>56%</meter>" should work like "<span>56%</span>"
bugzill...@webkit.org
May 12, 2010, 9:29:43 PM5/12/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
Yael <yael....@nokia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #55016|0 |1
is obsolete| |
Attachment #55016|review? |
Flag| |
Attachment #55925| |review?
Flag| |
--- Comment #23 from Yael <yael....@nokia.com> 2010-05-12 18:29:41 PST ---
Created an attachment (id=55925)
--> (https://bugs.webkit.org/attachment.cgi?id=55925)
Update the patch with latest trunk changes.
Yael <yael....@nokia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #55016|0 |1
is obsolete| |
Attachment #55016|review? |
Flag| |
Attachment #55925| |review?
Flag| |
--- Comment #23 from Yael <yael....@nokia.com> 2010-05-12 18:29:41 PST ---
Created an attachment (id=55925)
--> (https://bugs.webkit.org/attachment.cgi?id=55925)
Update the patch with latest trunk changes.
bugzill...@webkit.org
May 12, 2010, 9:31:36 PM5/12/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
--- Comment #24 from WebKit Review Bot <webkit.r...@gmail.com> 2010-05-12 18:31:34 PST ---
Attachment 55925 did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/html/HTMLMeterElement.cpp:66: An else should appear on the same line as the preceding } [whitespace/newline] [4]
Total errors found: 1 in 38 files
If any of these errors are false positives, please file a bug against check-webkit-style.
--- Comment #24 from WebKit Review Bot <webkit.r...@gmail.com> 2010-05-12 18:31:34 PST ---
Attachment 55925 did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/html/HTMLMeterElement.cpp:66: An else should appear on the same line as the preceding } [whitespace/newline] [4]
Total errors found: 1 in 38 files
If any of these errors are false positives, please file a bug against check-webkit-style.
bugzill...@webkit.org
May 12, 2010, 9:33:55 PM5/12/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
Yael <yael....@nokia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #55925|0 |1
Yael <yael....@nokia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
is obsolete| |
Attachment #55925|review? |
Flag| |
Attachment #55926| |review?
Flag| |
--- Comment #25 from Yael <yael....@nokia.com> 2010-05-12 18:33:52 PST ---
Created an attachment (id=55926)
--> (https://bugs.webkit.org/attachment.cgi?id=55926)
Fix style issue
bugzill...@webkit.org
May 12, 2010, 11:02:45 PM5/12/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
--- Comment #26 from Kent Tamura <tk...@chromium.org> 2010-05-12 20:02:43 PST ---
(From update of attachment 55926)
WebCore/rendering/RenderTheme.cpp:924
+ valueRect.setLocation(FloatPoint(innerRect.x(), innerRect.y() + (max - value) * scale));
Need static_cast<float>() because max, value and scale are double.
WebCore/rendering/RenderTheme.cpp:925
+ valueRect.setSize(FloatSize(innerRect.width(), (value - min) * scale));
ditto.
WebCore/rendering/RenderTheme.cpp:929
+ valueRect.setLocation(FloatPoint(innerRect.x() + (max - value) * scale, innerRect.y()));
ditto.
WebCore/rendering/RenderTheme.cpp:930
+ valueRect.setSize(FloatSize((value - min) * scale, innerRect.height()));
ditto.
WebCore/rendering/RenderTheme.cpp:935
+ valueRect.setSize(FloatSize((value - min) * scale, innerRect.height()));
ditto.
(In reply to comment #22)
So your code mean that :optional is always applied to <meter>.
I don't think we need to make it true for elements without form values.
If you'd like to avoid form validation for <meter>, override recalcWillValidate(). See HTMLFieldSetElement.h.
> > Could you add a test confirming that <meter> tag behavior is not changed on platforms without ENABLE_METER_TAG?
> > For example, "<meter>56%</meter>" should work like "<span>56%</span>"
> This was discussed before, in https://bugs.webkit.org/show_bug.cgi?id=35937. There is no mechanism for adding conditions to html.css, so the new style definition will affect ports with this flag turned off. There is precedence with other new tags e.g. datagrid for doing the same.
I see. How about parsing test?
I tried your patch on Mac without ENABLE_METER_TAG, and found <meter> tags were completely disappeared.
--- Comment #26 from Kent Tamura <tk...@chromium.org> 2010-05-12 20:02:43 PST ---
(From update of attachment 55926)
WebCore/rendering/RenderTheme.cpp:924
+ valueRect.setLocation(FloatPoint(innerRect.x(), innerRect.y() + (max - value) * scale));
Need static_cast<float>() because max, value and scale are double.
WebCore/rendering/RenderTheme.cpp:925
+ valueRect.setSize(FloatSize(innerRect.width(), (value - min) * scale));
ditto.
WebCore/rendering/RenderTheme.cpp:929
+ valueRect.setLocation(FloatPoint(innerRect.x() + (max - value) * scale, innerRect.y()));
ditto.
WebCore/rendering/RenderTheme.cpp:930
+ valueRect.setSize(FloatSize((value - min) * scale, innerRect.height()));
ditto.
WebCore/rendering/RenderTheme.cpp:935
+ valueRect.setSize(FloatSize((value - min) * scale, innerRect.height()));
ditto.
(In reply to comment #22)
> (In reply to comment #21)
> > (From update of attachment 55016 [details] [details])
> > WebCore/html/HTMLMeterElement.h:54
> > + virtual bool isOptionalFormControl() const { return true; }
> > This seems curious. Why do we need to return false for isOptionalFormControl()?
> >
> From http://dev.w3.org/html5/spec/Overview.html#attr-input-required , the required attribute applies only to input (and textarea) elements. All other form control elements return true.
isOptionalFormControl() is used only for checking whether :optional CSS selector is applied or not.
> > + virtual bool isOptionalFormControl() const { return true; }
> > This seems curious. Why do we need to return false for isOptionalFormControl()?
> >
> From http://dev.w3.org/html5/spec/Overview.html#attr-input-required , the required attribute applies only to input (and textarea) elements. All other form control elements return true.
So your code mean that :optional is always applied to <meter>.
I don't think we need to make it true for elements without form values.
If you'd like to avoid form validation for <meter>, override recalcWillValidate(). See HTMLFieldSetElement.h.
> > Could you add a test confirming that <meter> tag behavior is not changed on platforms without ENABLE_METER_TAG?
> > For example, "<meter>56%</meter>" should work like "<span>56%</span>"
> This was discussed before, in https://bugs.webkit.org/show_bug.cgi?id=35937. There is no mechanism for adding conditions to html.css, so the new style definition will affect ports with this flag turned off. There is precedence with other new tags e.g. datagrid for doing the same.
I tried your patch on Mac without ENABLE_METER_TAG, and found <meter> tags were completely disappeared.
bugzill...@webkit.org
May 13, 2010, 8:27:39 PM5/13/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
--- Comment #27 from Yael <yael....@nokia.com> 2010-05-13 17:27:37 PST ---
(In reply to comment #26)
Could you please share the test page you were using? I tried <meter>%56</meter> and that showed in safari the same as <span>%56</span>.
The only problem I saw was the style, and that was due to the webkit-appearance.
thanks.
--- Comment #27 from Yael <yael....@nokia.com> 2010-05-13 17:27:37 PST ---
(In reply to comment #26)
> I see. How about parsing test?
> I tried your patch on Mac without ENABLE_METER_TAG, and found <meter> tags were completely disappeared.
Thank you for the review.
> I tried your patch on Mac without ENABLE_METER_TAG, and found <meter> tags were completely disappeared.
Could you please share the test page you were using? I tried <meter>%56</meter> and that showed in safari the same as <span>%56</span>.
The only problem I saw was the style, and that was due to the webkit-appearance.
thanks.
bugzill...@webkit.org
May 13, 2010, 9:42:00 PM5/13/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
--- Comment #28 from Kent Tamura <tk...@chromium.org> 2010-05-13 18:41:58 PST ---
(In reply to comment #27)
- open an HTML page with <meter>56%</meter>
- Ctrl-Click on the 56%, and select "Inspect element"
- You'll see no <meter> around 56%
--- Comment #28 from Kent Tamura <tk...@chromium.org> 2010-05-13 18:41:58 PST ---
(In reply to comment #27)
> Could you please share the test page you were using? I tried <meter>%56</meter> and that showed in safari the same as <span>%56</span>.
> The only problem I saw was the style, and that was due to the webkit-appearance.
> thanks.
- run-safari --debug
> The only problem I saw was the style, and that was due to the webkit-appearance.
> thanks.
- open an HTML page with <meter>56%</meter>
- Ctrl-Click on the 56%, and select "Inspect element"
- You'll see no <meter> around 56%
bugzill...@webkit.org
May 14, 2010, 4:03:08 PM5/14/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
Yael <yael....@nokia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #55926|0 |1
Yael <yael....@nokia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
is obsolete| |
Attachment #55926|review? |
Flag| |
Attachment #56099| |review?
Flag| |
--- Comment #29 from Yael <yael....@nokia.com> 2010-05-14 13:03:06 PST ---
Created an attachment (id=56099)
--> (https://bugs.webkit.org/attachment.cgi?id=56099)
Patch addressing comment #26.
Tested on Safari and with this patch I do see the meter element in the web inspector.
bugzill...@webkit.org
May 15, 2010, 8:45:43 AM5/15/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
Kent Tamura <tk...@chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #56099|review? |review+
Flag| |
--- Comment #30 from Kent Tamura <tk...@chromium.org> 2010-05-15 05:45:41 PST ---
(From update of attachment 56099)
LayoutTests/platform/chromium/test_expectations.txt:2631
+ BUG37074 DEFER : fast/dom/HTMLMeterElement/set-meter-properties.html = TEXT
Please replace "BUG37074" with "BUGWK37074".
Kent Tamura <tk...@chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #56099|review? |review+
Flag| |
--- Comment #30 from Kent Tamura <tk...@chromium.org> 2010-05-15 05:45:41 PST ---
(From update of attachment 56099)
LayoutTests/platform/chromium/test_expectations.txt:2631
+ BUG37074 DEFER : fast/dom/HTMLMeterElement/set-meter-properties.html = TEXT
Please replace "BUG37074" with "BUGWK37074".
bugzill...@webkit.org
May 15, 2010, 12:57:25 PM5/15/10
to webkit-bo...@googlegroups.com
https://bugs.webkit.org/show_bug.cgi?id=38140
Yael <yael....@nokia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #31 from Yael <yael....@nokia.com> 2010-05-15 09:57:21 PST ---
Committed r59541: <http://trac.webkit.org/changeset/59541>
Yael <yael....@nokia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #31 from Yael <yael....@nokia.com> 2010-05-15 09:57:21 PST ---
Committed r59541: <http://trac.webkit.org/changeset/59541>
Reply all
Reply to author
Forward
0 new messages