Last Comment Bug 619996 - Remove nsISelectElement
: Remove nsISelectElement
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla5
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
Mentors:
Depends on:
Blocks: deCOM 604807
  Show dependency treegraph
 
Reported: 2010-12-17 13:36 PST by :Ms2ger (⌚ UTC+1/+2)
Modified: 2011-04-12 10:23 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part a: Remove unnecessary inclusions of and references to nsISelectElement.h (4.72 KB, patch)
2011-01-27 11:41 PST, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review
Part b: make nsHTMLOptionElement::GetSelect return nsHTMLSelectElement. (4.56 KB, patch)
2011-01-27 11:42 PST, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review
Part c: stop using QueryInterface on nsHTMLOptionElement::GetSelect's return value. (3.52 KB, patch)
2011-01-27 11:43 PST, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review
Part d: use nsHTMLSelectElement::FromContent in nsListControlFrame. (5.69 KB, patch)
2011-01-27 11:44 PST, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review
Part e: make nsSafeOptionListMutation::mSelect a nsHTMLSelectElement. (3.61 KB, patch)
2011-01-27 11:45 PST, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review
Part f: kill layout/forms/resources. (79.25 KB, patch)
2011-01-27 11:46 PST, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review
Part g: Kill nsISelectElement. (12.77 KB, patch)
2011-01-27 11:46 PST, :Ms2ger (⌚ UTC+1/+2)
bzbarsky: review+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2010-12-17 13:36:42 PST
As noted in bug 604807, we could remove it. Patches coming up.
Comment 1 :Ms2ger (⌚ UTC+1/+2) 2011-01-27 11:41:37 PST
Created attachment 507560 [details] [diff] [review]
Part a: Remove unnecessary inclusions of and references to nsISelectElement.h
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2011-01-27 11:42:21 PST
Created attachment 507561 [details] [diff] [review]
Part b: make nsHTMLOptionElement::GetSelect return nsHTMLSelectElement.
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2011-01-27 11:43:24 PST
Created attachment 507562 [details] [diff] [review]
Part c: stop using QueryInterface on nsHTMLOptionElement::GetSelect's return value.
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2011-01-27 11:44:06 PST
Created attachment 507563 [details] [diff] [review]
Part d: use nsHTMLSelectElement::FromContent in nsListControlFrame.
Comment 5 :Ms2ger (⌚ UTC+1/+2) 2011-01-27 11:45:13 PST
Created attachment 507564 [details] [diff] [review]
Part e: make nsSafeOptionListMutation::mSelect a nsHTMLSelectElement.
Comment 6 :Ms2ger (⌚ UTC+1/+2) 2011-01-27 11:46:03 PST
Created attachment 507566 [details] [diff] [review]
Part f: kill layout/forms/resources.
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2011-01-27 11:46:41 PST
Created attachment 507567 [details] [diff] [review]
Part g: Kill nsISelectElement.
Comment 8 Boris Zbarsky [:bz] (TPAC) 2011-03-16 08:20:04 PDT
Comment on attachment 507560 [details] [diff] [review]
Part a: Remove unnecessary inclusions of and references to nsISelectElement.h

r=me
Comment 9 Boris Zbarsky [:bz] (TPAC) 2011-03-16 08:23:10 PDT
Comment on attachment 507561 [details] [diff] [review]
Part b: make nsHTMLOptionElement::GetSelect return nsHTMLSelectElement.

>+    if (aContent &&
>+        aContent->NodeInfo()->Equals(nsGkAtoms::option, kNameSpaceID_XHTML))

As long as you're here:

  if (aContent && aContent->IsHTML(nsGkAtoms::option))

>+++ b/content/html/content/src/nsHTMLSelectElement.h

>+    if (!aContent ||
>+        !aContent->NodeInfo()->Equals(nsGkAtoms::select, kNameSpaceID_XHTML))
>+      return NULL;

Again, IsHTML().  And let's stick with nsnull until the mass-change.  ;)

r=me with the nits picked.
Comment 10 Boris Zbarsky [:bz] (TPAC) 2011-03-16 08:25:49 PDT
Comment on attachment 507562 [details] [diff] [review]
Part c: stop using QueryInterface on nsHTMLOptionElement::GetSelect's return value.

r=me
Comment 11 Boris Zbarsky [:bz] (TPAC) 2011-03-16 08:27:50 PDT
Comment on attachment 507563 [details] [diff] [review]
Part d: use nsHTMLSelectElement::FromContent in nsListControlFrame.

r=me, if you check that stuff like SetOptionsSelectedByIndex can't trigger script (e.g. via onchange events).  If it can, you need to hold strong refs in soe places here...
Comment 12 Boris Zbarsky [:bz] (TPAC) 2011-03-16 08:28:39 PDT
Comment on attachment 507564 [details] [diff] [review]
Part e: make nsSafeOptionListMutation::mSelect a nsHTMLSelectElement.

r=me
Comment 13 Boris Zbarsky [:bz] (TPAC) 2011-03-16 08:29:43 PDT
Comment on attachment 507566 [details] [diff] [review]
Part f: kill layout/forms/resources.

r=me; can't believe this was still around.
Comment 14 Boris Zbarsky [:bz] (TPAC) 2011-03-16 08:30:31 PDT
Comment on attachment 507567 [details] [diff] [review]
Part g: Kill nsISelectElement.

r=me

Note You need to log in before you can comment on or make changes to this bug.