Closed Bug 283686 Opened 15 years ago Closed 11 years ago
root element can not have display:none
72 bytes, text/html
6.61 KB, patch
|Details | Diff | Splinter Review|
177 bytes, text/html
5.38 KB, patch
|Details | Diff | Splinter Review|
For a testcase you can view attachment 133482 [details]. I will also attach a simple text/html testcase.
This asserts (because the undisplayed content has no parent). More importantly, this breaks focusing the content area (using the testcase in this bug) -- I can neither tab to the content area, nor focus it via click. That seems like a problem....
*** Bug 313010 has been marked as a duplicate of this bug. ***
Also see the changes made in bug 454361.
Is there a reason we don't want to hash null in nsFrameManager? This seems to fix Testcase #2, although not the focus problem Boris noted.
Nominating this for blocking1.9.1 because, given the changes we made in bug 454361, this can now cause the root element to become forever-undisplayed (instead of forever-displayed). In other words, we now support making the root element display:none, but once it's there, you can't get it back. That seems like a bad state to be in, and I could even imagine it breaking sites if people are using toggling display on the root as a workaround for another browser.
I think the patch is fine if it were not for bug 463511 where we have a call with content from within an anonymous subtree (GetParent() is null in that case too). The focus problem is a separate bug (pres shell event delivery), I have a fix for that locally.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Whiteboard: [CSS1-5.6.1] → [CSS1-5.6.1][has wip patch]
(In reply to comment #8) > I think the patch is fine if it were not for bug 463511 where we have > a call with content from within an anonymous subtree (GetParent() is > null in that case too). I don't understand. Does this patch make that bug worse?
Maybe in SetUndisplayedContent we should just do a runtime check for if it's the root content? That seems like it could also be useful in the future depending on how we implement XBL2 (or even how svg:use is implemented).
(by runtime check, I mean changing what's now an assertion to be a real test)
Mats, Boris, David, can one of you own this? I'd work on it except I don't understand what the XBL issue is here.
I'll take it.
Assignee: nobody → dbaron
So, actually, I don't think anything goes wrong even with bug 463511. The undisplayed map tolerates multiple children of the same content node being display:none; now it'll just be as though there are multiple children of "null" being display:none. I don't see anything wrong with that. I think we should change the assertion to fail if mPresShell or mPresShell->GetDocument() are null (rather than succeed and potentially cover a problem), though. I'll post the patch with that change.
This is attachment 345755 [details] [diff] [review], the assertion change, and three reftests (static, dynamic from none, and dynamic to none). This patch itself fixes the second reftest.
Whiteboard: [CSS1-5.6.1][has wip patch] → [CSS1-5.6.1][needs landing]
Fixed: http://hg.mozilla.org/mozilla-central/rev/7bf1d1b7f3b8 I split off the focus problem into bug 472980, and I'm marking this bug as fixed.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [CSS1-5.6.1][needs landing] → [CSS1-5.6.1][needs 1.9.1 landing]
Target Milestone: --- → mozilla1.9.2a1
Interestingly - to me at least - the bug stops Ctrl+U from working. Have to go View->Page Source via the menus. (At least I'm hoping it IS this bug that does that.) Is this a consequence you'd expect of this bug? It sounds to me like it is.
Sounds exactly like bug 472980, which is separate from this bug.
Whiteboard: [CSS1-5.6.1][needs 1.9.1 landing] → [CSS1-5.6.1]
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
Verified on trunk and 1.9.1 with builds on OS X and Windows: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204020327 Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090203 Minefield/3.2a1pre
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.