Closed Bug 826632 Opened 7 years ago Closed 7 years ago

Merge nsIViewManager into nsViewManager and clean up a bit

Categories

(Core :: Layout, defect)

18 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: roc, Assigned: roc)

Details

Attachments

(7 files)

No description provided.
Attachment #697846 - Flags: review?(tnikkel) → review+
Attachment #697847 - Flags: review?(tnikkel) → review+
Attachment #697848 - Flags: review?(tnikkel) → review+
Attachment #697849 - Flags: review?(tnikkel) → review+
Comment on attachment 697850 [details] [diff] [review]
Part 5: nsViewManager methods do not need to be virtual and most of the ones that return nsresult don't need to

>-NS_IMETHODIMP nsViewManager::RemoveChild(nsView *aChild)
>+void
>+nsViewManager::RemoveChild(nsView *aChild)
> {
>-  NS_ENSURE_ARG_POINTER(aChild);
>+  NS_ASSERTION(aChild, "aChild must not be null");

NS_ENSURE_ARG_POINTER returns if the argument is null, so this is a change of behaviour. Intentional?
Attachment #697850 - Flags: review?(tnikkel) → review+
Attachment #697851 - Flags: review?(tnikkel) → review+
Attachment #697852 - Flags: review?(tnikkel) → review+
(In reply to Timothy Nikkel (:tn) from comment #8)
> NS_ENSURE_ARG_POINTER returns if the argument is null, so this is a change
> of behaviour. Intentional?

Yes. It doesn't matter.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13)
> https://tbpl.mozilla.org/?tree=Try&rev=8103d9167be4

So part 1 did not cause the test failures, but the patches up to part 7 did.

Let's try up to part 4: https://tbpl.mozilla.org/?tree=Try&rev=32f1edb2e710
I think I found the bug; in part 5, in nsViewManager::DispatchEvent, I removed a return statement that I shouldn't have.
https://tbpl.mozilla.org/?tree=Try&rev=a29c06d23a80
You need to log in before you can comment on or make changes to this bug.