Closed
Bug 756045
Opened 12 years ago
Closed 12 years ago
Turn "ASSERTION: anonymous nodes should not be in child lists" into an NS_WARNING
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: ayg, Assigned: ayg)
References
Details
Attachments
(1 file, 1 obsolete file)
2.48 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
See bug 751842 comment 32. We know about the assertion, and we aren't going to fix it anytime soon. It gets hit a million time during editor tests, which both slows them down and clogs the logs.
Flags: in-testsuite-
Assignee | ||
Comment 1•12 years ago
|
||
This might need more test updates.
Attachment #624687 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland-try:-u all]
Assignee | ||
Updated•12 years ago
|
Assignee: ayg → nobody
Component: Editor → Style System (CSS)
QA Contact: editor → style-system
Updated•12 years ago
|
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]
Assignee | ||
Comment 2•12 years ago
|
||
Bah, autoland is flaky. Manual try push: https://tbpl.mozilla.org/?tree=Try&rev=eb9a4fff4e2f
Whiteboard: [autoland-in-queue]
Assignee | ||
Comment 3•12 years ago
|
||
Assignee: nobody → ayg
Attachment #624687 -
Attachment is obsolete: true
Attachment #624687 -
Flags: review?(bzbarsky)
Attachment #624724 -
Flags: review?(bzbarsky)
Comment 4•12 years ago
|
||
Why are editor tests hitting this? The manual insanity it does with the resize handles or something?
Assignee | ||
Comment 5•12 years ago
|
||
I'll leave Ehsan to answer that . . .
Comment 6•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #4) > Why are editor tests hitting this? The manual insanity it does with the > resize handles or something? There's a ton of cases where you can hit this from content script. Calling .focus() on an abs-pos editable element is one example.
Comment 7•12 years ago
|
||
> Calling .focus() on an abs-pos editable element is one example.
Er.... why does that hit this assert?
Basically, if hitting this case is nominally a bug, we should document here why this is a warning, not an assert, and document at the buggy callsites that once they're fixed this should become an assert.
If this case is not nominally a bug, we shouldn't have a warning.
Comment 8•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #7) > > Calling .focus() on an abs-pos editable element is one example. > > Er.... why does that hit this assert? Because that triggers the resizing and positioning controls to be displayed on focus, and we hit this assertion once for every one of those controls. > Basically, if hitting this case is nominally a bug, we should document here > why this is a warning, not an assert, and document at the buggy callsites > that once they're fixed this should become an assert. > > If this case is not nominally a bug, we shouldn't have a warning. The fact that we hit this assert is because we inject those controls in a very bad way, but that bad way works as expected, so this is technically not a bug. It's just ugly things that we should not be doing. The proper way of doing this is to add those as anonymous abs-pos children of the element they're supposed to control, but doing that requires us to enable all sorts of frames to be abs-pos containers, and also make sure that elements such as images can deal with anonymous children, so it is a longer term project.
Comment 9•12 years ago
|
||
Oh, I'd somehow misread "editable element" as "text input". I have _no_ idea why, and I'm sorry about the confusion. So yes, this is all about the resize handles. Aryeh, please add comments to the code where the warning is pointing that out, and please add code comments to the caller of ContentRemoved caller in editor about reinstating the assert once that caller is fixed.
Comment 10•12 years ago
|
||
Comment on attachment 624724 [details] [diff] [review] Patch v2, try failure fixed r=me with my requested comment fixes.
Attachment #624724 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 11•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8be8a188bd60
Target Milestone: --- → mozilla15
Comment 12•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8be8a188bd60
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•