root element can not have display:none

VERIFIED FIXED in mozilla1.9.2a1

Status

()

P2
normal
VERIFIED FIXED
14 years ago
4 months ago

People

(Reporter: annevk, Assigned: dbaron)

Tracking

({css1, testcase, verified1.9.1})

Trunk
mozilla1.9.2a1
css1, testcase, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [CSS1-5.6.1])

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

14 years ago
For a testcase you can view attachment 133482 [details]. I will also attach a simple
text/html testcase.
(Reporter)

Comment 1

14 years ago
Created attachment 175578 [details]
testcase
Created attachment 175627 [details] [diff] [review]
Partial patch

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....

Comment 3

13 years ago
*** Bug 313010 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Whiteboard: [CSS1-5.6.1]
Created attachment 345744 [details]
Testcase #2
Created attachment 345755 [details] [diff] [review]
wip

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.
Flags: blocking1.9.1?
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
(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).
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.
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.
Created attachment 356096 [details] [diff] [review]
patch for dynamic changes

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.
Attachment #345755 - Attachment is obsolete: true
Attachment #356096 - Flags: superreview?(bzbarsky)
Attachment #356096 - Flags: review?(bzbarsky)
Attachment #356096 - Flags: superreview?(bzbarsky)
Attachment #356096 - Flags: superreview+
Attachment #356096 - Flags: review?(bzbarsky)
Attachment #356096 - Flags: review+
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
Last Resolved: 10 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

Comment 17

10 years ago
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.
Depends on: 473678
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7b5240017a6d
Keywords: fixed1.9.1
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
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Keywords: fixed1.9.1 → verified1.9.1
Target Milestone: mozilla1.9.1b3 → mozilla1.9.2a1

Updated

4 months ago
Product: Core → Core Graveyard
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.