|Summary:||Implement HTML5 hidden attribute|
|Product:||WebKit||Reporter:||Maciej Stachowiak <mjs>|
|Component:||New Bugs||Assignee:||Maciej Stachowiak <mjs>|
|Severity:||Normal||CC:||abarth, commit-queue, emacemac7, eric, mike, Ms2ger, tony, webkit.review.bot|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
|Bug Depends on:||40517|
Description Maciej Stachowiak 2010-06-11 18:12:06 PDT
Implement HTML5 hidden attribute
Comment 2 Maciej Stachowiak 2010-06-11 18:28:35 PDT
Committed r61052: <http://trac.webkit.org/changeset/61052>
Comment 3 WebKit Review Bot 2010-06-11 19:03:18 PDT
http://trac.webkit.org/changeset/61052 might have broken SnowLeopard Intel Release (Tests)
Comment 4 Eric Seidel (no email) 2010-06-11 20:57:30 PDT
Bots are borked.
Comment 5 Eric Seidel (no email) 2010-06-11 23:16:09 PDT
Been borked for 5 hours. Marked for rollout to let the cq continue.
Comment 6 Eric Seidel (no email) 2010-09-02 13:51:37 PDT
I guess this was rolled out? Was it rolled back in?
Comment 7 Peter Beverloo 2010-12-03 02:28:02 PST
Created attachment 75482 [details] Patch Re-baselined Maciej patch after brief discussion on IRC. Left the changelog messages intact as I didn't change anything. Something I did notice was that HTMLEmbedElement.cpp (line 108) already implemented the hidden attribute in a slightly different way, by setting the width and height to zero. I haven't changed this because I don't know the rationale behind the implementation, but given the following line from the HTML5 specification it might be wise to create a (follow up?) patch to update the element's implementation. > The embed element is unaffected by the CSS 'display' property. The selected > plugin is instantiated even if the element is hidden with a 'display:none' > CSS style. http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#the-embed-element
Comment 8 WebKit Commit Bot 2010-12-03 13:21:42 PST
Comment on attachment 75482 [details] Patch Rejecting patch 75482 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']" exit_code: 2 Last 500 characters of output: ................................................ fast/eventsource .. fast/fast-mobile-scrolling .. fast/files ....... fast/flexbox ................................... fast/forms .................................................................................................... fast/forms/caret-rtl.html -> failed Exiting early after 1 failures. 8025 tests run. 135.50s total testing time 8024 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 4 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/6845035
Comment 9 Peter Beverloo 2010-12-03 14:15:07 PST
Created attachment 75542 [details] Patch The style attribute of fast/forms/caret-rtl.html was closed too early, causing the outline- and overflow properties to be threated as attributes. Since the overflow property had "hidden" as its attribute, the whole edit-layer disappeared. I have modified the test to show the outline, but have removed the overflow to maintain the current results.
Comment 10 Peter Beverloo 2010-12-06 00:14:13 PST
Created attachment 75654 [details] Patch Fixed the patch.
Comment 11 Kent Tamura 2010-12-07 01:36:48 PST
Comment on attachment 75654 [details] Patch Looks good.
Comment 12 WebKit Commit Bot 2010-12-07 11:40:08 PST
Comment on attachment 75654 [details] Patch Rejecting patch 75654 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-sf', 'build', '--no-clean', '--no-update', '--build-style=both']" exit_code: 2 Last 500 characters of output: 2 CompileC /Projects/CommitQueue/WebKitBuild/WebCore.build/Debug/WebCore.build/Objects-normal/x86_64/JSEntriesCallback.o /Projects/CommitQueue/WebKitBuild/Debug/DerivedSources/WebCore/JSEntriesCallback.cpp normal x86_64 c++ com.apple.compilers.gcc.4_2 CompileC /Projects/CommitQueue/WebKitBuild/WebCore.build/Debug/WebCore.build/Objects-normal/x86_64/JSEntity.o /Projects/CommitQueue/WebKitBuild/Debug/DerivedSources/WebCore/JSEntity.cpp normal x86_64 c++ com.apple.compilers.gcc.4_2 (3 failures) Full output: http://queues.webkit.org/results/6751097
Comment 13 Ryosuke Niwa 2010-12-07 11:55:00 PST
Comment on attachment 75654 [details] Patch Let's try again.
Comment 14 WebKit Commit Bot 2010-12-07 14:02:15 PST
Comment on attachment 75654 [details] Patch Clearing flags on attachment: 75654 Committed r73459: <http://trac.webkit.org/changeset/73459>
Comment 15 Tony Chang 2010-12-07 15:21:09 PST
This caused the focus ring around the div to no longer be painted in fast/forms/caret-rtl.html. Is that expected? Here's the pixel results on Chromium: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&showLargeExpectations=true&tests=fast%2Fforms%2Fcaret-rtl.html&master=ChromiumWebkit
Comment 16 Tony Chang 2010-12-07 16:16:32 PST
(In reply to comment #15) > This caused the focus ring around the div to no longer be painted in fast/forms/caret-rtl.html. Is that expected? To answer my own question: yes, because the content editable div now has an outline style. I'll rebaseline.
Comment 17 Eric Seidel (no email) 2010-12-14 01:29:02 PST
Comment on attachment 75482 [details] Patch Cleared Maciej Stachowiak's review+ from obsolete attachment 75482 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 18 Eric Seidel (no email) 2010-12-20 22:49:09 PST
Please use a separate bug for regressions.
Comment 19 Eric Seidel (no email) 2011-06-01 16:35:09 PDT
Um... Looking at: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#the-embed-element I no longer see a "hidden" attribute on <embed> are we sure it's still a part of the spec? This change caused "regression" bug 56575. If "hidden" is no longer an attribute on <embed> seems we should roll out this patch.
Comment 20 Eric Seidel (no email) 2011-06-01 16:36:55 PDT
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#the-object-element also doesn't mention anything about a "hidden" attribute. I think this patch should be rolled out.
Comment 21 Eric Seidel (no email) 2011-06-01 16:38:24 PDT
Nevermind, hidden is a global attribute: http://www.whatwg.org/specs/web-apps/current-work/multipage/editing.html#the-hidden-attribute