| Summary: | :active style is not cleared when its display property is set to none before mouse released. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sanghyup Lee <sh53.lee> | ||||||||
| Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | commit-queue, esprehn+autocc, gyuyoung.kim, jinwoo7.song, kangil.han, kling, ryuan.choi | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Sanghyup Lee
2014-02-27 20:33:48 PST
Created attachment 225433 [details]
Patch
Comment on attachment 225433 [details] Patch It's great that you are fixing this bug, but we should add a layout test so it doesn't regress again. http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree Created attachment 225542 [details]
Patch
Comment on attachment 225542 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225542&action=review > Source/WebCore/dom/Document.cpp:5839 > + if (!oldActiveElement->renderer()) > + oldActiveElement->setActive(false); This code change seems like only a partial fix. Last time Document::updateHoverActiveState was called, we called setActive(true) on multiple elements. It seems that elements that were ancestors of the main active one will have the same bug as before. We should construct test cases that cover that and fix the whole problem instead of patching over the most obvious surface part of the problem. There’s no real need for the !oldActiveElement->renderer() check. It would be harmless and clearer to call oldActiveElement->setActive(false) unconditionally. It’s hard to understand why we would do this only for an element without a renderer. Slightly ugly to fetch the renderer twice, here and for the loop just below it. Created attachment 225580 [details]
Patch
Comment on attachment 225580 [details]
Patch
r=me
Comment on attachment 225580 [details]
Patch
cq=me. It looks latest patch is fixed according to Darin's comment.
Comment on attachment 225580 [details] Patch Clearing flags on attachment: 225580 Committed r165037: <http://trac.webkit.org/changeset/165037> All reviewed patches have been landed. Closing bug. |