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)
:
: Andrew Overholt [:overholt]
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 User image :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 User image :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 User image :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 User image :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 User image :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 User image :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 User image :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 User image :Ms2ger (⌚ UTC+1/+2) 2011-01-27 11:46:41 PST
Created attachment 507567 [details] [diff] [review]
Part g: Kill nsISelectElement.
Comment 8 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image Boris Zbarsky [:bz] (still a bit busy) 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.