Bug 40511 - Implement HTML5 hidden attribute
: Implement HTML5 hidden attribute
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: Other Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
: 40517
:
  Show dependency treegraph
 
Reported: 2010-06-11 18:12 PST by
Modified: 2011-06-01 16:38 PST (History)


Attachments
Patch (5.51 KB, patch)
2010-06-11 18:17 PST, Maciej Stachowiak
ojan: review+
Review Patch | Details | Formatted Diff | Diff
Patch (5.52 KB, patch)
2010-12-03 02:28 PST, Peter Beverloo
no flags Review Patch | Details | Formatted Diff | Diff
Patch (6.40 KB, patch)
2010-12-03 14:15 PST, Peter Beverloo
no flags Review Patch | Details | Formatted Diff | Diff
Patch (6.28 KB, patch)
2010-12-06 00:14 PST, Peter Beverloo
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-06-11 18:12:06 PST
Implement HTML5 hidden attribute
------- Comment #1 From 2010-06-11 18:17:17 PST -------
Created an attachment (id=58533) [details]
Patch
------- Comment #2 From 2010-06-11 18:28:35 PST -------
Committed r61052: <http://trac.webkit.org/changeset/61052>
------- Comment #3 From 2010-06-11 19:03:18 PST -------
http://trac.webkit.org/changeset/61052 might have broken SnowLeopard Intel Release (Tests)
------- Comment #4 From 2010-06-11 20:57:30 PST -------
Bots are borked.
------- Comment #5 From 2010-06-11 23:16:09 PST -------
Been borked for 5 hours.  Marked for rollout to let the cq continue.
------- Comment #6 From 2010-09-02 13:51:37 PST -------
I guess this was rolled out?  Was it rolled back in?
------- Comment #7 From 2010-12-03 02:28:02 PST -------
Created an attachment (id=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 From 2010-12-03 13:21:42 PST -------
(From update of attachment 75482 [details])
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 From 2010-12-03 14:15:07 PST -------
Created an attachment (id=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 From 2010-12-06 00:14:13 PST -------
Created an attachment (id=75654) [details]
Patch

Fixed the patch.
------- Comment #11 From 2010-12-07 01:36:48 PST -------
(From update of attachment 75654 [details])
Looks good.
------- Comment #12 From 2010-12-07 11:40:08 PST -------
(From update of attachment 75654 [details])
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 From 2010-12-07 11:55:00 PST -------
(From update of attachment 75654 [details])
Let's try again.
------- Comment #14 From 2010-12-07 14:02:15 PST -------
(From update of attachment 75654 [details])
Clearing flags on attachment: 75654

Committed r73459: <http://trac.webkit.org/changeset/73459>
------- Comment #15 From 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 From 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 From 2010-12-14 01:29:02 PST -------
(From update of attachment 75482 [details])
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 From 2010-12-20 22:49:09 PST -------
Please use a separate bug for regressions.
------- Comment #19 From 2011-06-01 16:35:09 PST -------
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 From 2011-06-01 16:36:55 PST -------
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 From 2011-06-01 16:38:24 PST -------
Nevermind, hidden is a global attribute:
http://www.whatwg.org/specs/web-apps/current-work/multipage/editing.html#the-hidden-attribute