Closed
Bug 1162765
Opened 9 years ago
Closed 4 years ago
Convert nsGenericHTMLFormElement::mForm to a strong reference
Categories
(Core :: DOM: Core & HTML, defect, P5)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: ehsan.akhgari, Unassigned)
References
Details
Attachments
(2 files)
4.47 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter Review |
5.60 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter Review |
We have a cycle collector now, so this should be fine!
Reporter | ||
Comment 1•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e61aaeeedc4
Attachment #8603157 -
Flags: review?(continuation)
Reporter | ||
Comment 2•9 years ago
|
||
Attachment #8603159 -
Flags: review?(continuation)
Updated•9 years ago
|
Attachment #8603157 -
Flags: review?(continuation) → review+
Comment 3•9 years ago
|
||
Comment on attachment 8603159 [details] [diff] [review] Part 2: Convert nsGenericHTMLFormElement::mFieldSet to a strong reference Review of attachment 8603159 [details] [diff] [review]: ----------------------------------------------------------------- It looks like this mDependentElements could be strong now, too. Please at least file a bug for that if you think that makes sense.
Attachment #8603159 -
Flags: review?(continuation) → review+
Reporter | ||
Comment 4•9 years ago
|
||
Filed bug 1163148 as a follow-up.
https://hg.mozilla.org/integration/mozilla-inbound/rev/c41fa4e8dfb2 https://hg.mozilla.org/integration/mozilla-inbound/rev/18886e5d06df
Backed out for being the likely cause of frequent win debug m2 orange: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/e6d93b374a9b remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/673edf1e9272 https://treeherder.mozilla.org/logviewer.html#?job_id=9712720&repo=mozilla-inbound
Flags: needinfo?(ehsan)
Reporter | ||
Comment 7•9 years ago
|
||
The cause of these assertions is this code: <https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLInputElement.cpp#4564>. Here we look at the element's type and mForm to determine whether to remove it from the radio group. The problem is that after the node gets unlinked, mForm will be null, and we have no way of knowing whether it was previously assigned to or not. Andrew, can you think of a clean way to check whether a node has been unlinked during UnbindFromTree? I can't think of a much cleaner way to fix this except for maintaining a flag in nsGenericHTMLFormElement indicating whether mForm has ever been assigned to, which sucks... :/
Flags: needinfo?(ehsan) → needinfo?(continuation)
Comment 8•9 years ago
|
||
Olli, do you have some ideas? You are more familiar with all of this node unlinking stuff.
Flags: needinfo?(bugs)
Comment 9•9 years ago
|
||
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #7) > Andrew, can you think of a clean way to check whether a node has been > unlinked during UnbindFromTree? What does this even mean ;) But no, we don't really have any flags indicating whether something has been unlinked, if that is what the question is about. > I can't think of a much cleaner way to fix > this except for maintaining a flag in nsGenericHTMLFormElement indicating > whether mForm has ever been assigned to, which sucks... :/ Yeah, or just keep mForm around? Don't really see other ways. There needs to be some flag for this case (either bool or pointer to form).
Flags: needinfo?(bugs)
Updated•9 years ago
|
Flags: needinfo?(continuation)
Reporter | ||
Comment 10•9 years ago
|
||
Is it OK to keep mForm around? I thought that Unlink() is supposed to remove the strong refs that Traverse() discovers. If that is OK, it's probably an acceptable solution here.
Comment 11•9 years ago
|
||
well, I was thinking to keep mForm as a raw reference and rely on the current way to clear it. (in other words, wontfix this bug)
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(ehsan)
Comment 12•6 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046 Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5. If you have questions, please contact :mdaly.
Priority: -- → P5
Assignee | ||
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
Reporter | ||
Updated•4 years ago
|
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(ehsan)
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•