remove IServiceProvider from nsAccessNodeWrap and make it a tear off

RESOLVED FIXED in mozilla22

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Tracking

unspecified
mozilla22
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
presumably it isn't needed most of the time and when its used its for a short time, so it doesn't really make sense for lots of objects to implement it.
(Assignee)

Comment 1

5 years ago
Created attachment 703754 [details] [diff] [review]
implement IServiceProvider with a taer off
Attachment #703754 - Flags: review?(surkov.alexander)

Comment 2

5 years ago
Comment on attachment 703754 [details] [diff] [review]
implement IServiceProvider with a taer off

Review of attachment 703754 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/msaa/ServiceProvider.cpp
@@ +1,5 @@
> +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> +/* vim: set ts=8 sts=2 et sw=2 tw=80: */
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + *  * License, v. 2.0. If a copy of the MPL was not distributed with this
> + *   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

nit: please check if those whitespaces between * and * in last two lines are expected

@@ +24,5 @@
> +IMPL_IUNKNOWN_QUERY_HEAD(ServiceProvider)
> +  IMPL_IUNKNOWN_QUERY_IFACE(IServiceProvider)
> +  return mAccessible->QueryInterface(aIID, aInstancePtr);
> +A11Y_TRYBLOCK_END
> +  }

not nice, but ok, you could do
if (mAccessible)
  return mAccessible->QI;
IMPL_IUNKNOWN_QUERY_TAIL

but mAccessible shouldn't be null in practice

@@ +31,5 @@
> +// IServiceProvider
> +
> +STDMETHODIMP
> +ServiceProvider::QueryService(REFGUID aGuidService, REFIID aIID,
> +                             void** aInstancePtr)

nit: pls indent this line properly

@@ +60,5 @@
> +  if (aGuidService == SID_IAccessibleContentDocument) {
> +    if (aIID != IID_IAccessible)
> +      return E_NOINTERFACE;
> +
> +    nsCOMPtr<nsIDocShellTreeItem> docShellTreeItem = 

nit: whitespace in the end of line

@@ +114,5 @@
> +   *   const GUID unused;
> +   *   pServProv->QueryService(unused, IID_ISimpleDOMDocument, (void**)&pAccDoc);
> +   *   pServProv->Release();
> +   * }
> +   */

nit: please use single line comment style but basically this comment is obsolete

::: accessible/src/msaa/ServiceProvider.h
@@ +1,5 @@
> +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> +/* vim: set ts=8 sts=2 et sw=2 tw=80: */
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + *  * License, v. 2.0. If a copy of the MPL was not distributed with this
> + *   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

same here, it go with http://www.mozilla.org/MPL/headers/
Attachment #703754 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 3

5 years ago
> @@ +114,5 @@
> > +   *   const GUID unused;
> > +   *   pServProv->QueryService(unused, IID_ISimpleDOMDocument, (void**)&pAccDoc);
> > +   *   pServProv->Release();
> > +   * }
> > +   */
> 
> nit: please use single line comment style but basically this comment is
> obsolete

mind if I just nuke it then?

Updated

5 years ago
Summary: remove IServiceProvider from nsAccessNodeWrap and make it a taer off → remove IServiceProvider from nsAccessNodeWrap and make it a tear off

Comment 4

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #3)

> > nit: please use single line comment style but basically this comment is
> > obsolete
> 
> mind if I just nuke it then?

it seems the best option :)
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb29fef8cd2e
https://hg.mozilla.org/mozilla-central/rev/bb29fef8cd2e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
See Also: → bug 1284314
You need to log in before you can comment on or make changes to this bug.