1414230 - Clean up markup table lookup code

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement, P5)

Description

[:Paolo Amadini]

From bug 1403231 comment 26:
- mMarkupMaps -> mHTMLMap
- mXULMarkupMaps -> mXULMap
- Replace functions living in a separate file on lambdas
- Consolidate the XULMAP and MARKUPMAP macros

Comment 1

[alexander :surkov (:asurkov)]

thanks for filing it

Comment 2

[:Paolo Amadini]

Yura, this bug and in particular the conversion to lambdas may be useful for bug 1428930.

Do you think you may find some time to work on it, or at least provide a starting point?

Comment 3

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to :Paolo Amadini from comment #2)
> Yura, this bug and in particular the conversion to lambdas may be useful for
> bug 1428930.
> 
> Do you think you may find some time to work on it, or at least provide a
> starting point?

I think Alex would definitely have feedback, I'm a little swamped right now with DevTools work, moving ni? to him.

Comment 4

[:Paolo Amadini]

Cool, thanks for the quick reply!

Comment 5

[alexander :surkov (:asurkov)]

Bug 1403231#c26 and this bug summary could serve as a starting point, any particular questions I could help to answer to?

Comment 6

[:Paolo Amadini]

That actually looks enough for someone familiar with modern C++ to start working on this bug.

Comment 7

[:Paolo Amadini]

Attachment: Bug 1414230 - Part 1 - Align XUL and HTML markup table names.

Review commit: https://reviewboard.mozilla.org/r/217868/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/217868/

Comment 8

[:Paolo Amadini]

Attachment: Bug 1414230 - Part 2 - Simplify XUL markup map creation.

Review commit: https://reviewboard.mozilla.org/r/217870/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/217870/

Comment 9

[Code Review Bot [:reviewbot]]

Comment on attachment 8948394 [details]
Bug 1414230 - Part 1 - Align XUL and HTML markup table names.

https://reviewboard.mozilla.org/r/217868/#review223622


Static analysis found 2 defects in this patch.
 - 2 defects found by clang-tidy

You can run this analysis locally with:
 - `./mach static-analysis check path/to/file.cpp` (C/C++)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: accessible/base/nsAccessibilityService.cpp:1353
(Diff revision 1)
>    if (!eventListenerService)
>      return false;
>  
>    eventListenerService->AddListenerChangeListener(this);
>  
> -  for (uint32_t i = 0; i < ArrayLength(sMarkupMapList); i++)
> +  for (uint32_t i = 0; i < ArrayLength(sHTMLMarkupMapList); i++)

Warning: Use range-based for loop instead [clang-tidy: modernize-loop-convert]

::: accessible/base/nsAccessibilityService.cpp:1357
(Diff revision 1)
>  
> -  for (uint32_t i = 0; i < ArrayLength(sMarkupMapList); i++)
> -    mMarkupMaps.Put(*sMarkupMapList[i].tag, &sMarkupMapList[i]);
> +  for (uint32_t i = 0; i < ArrayLength(sHTMLMarkupMapList); i++)
> +    mHTMLMarkupMap.Put(*sHTMLMarkupMapList[i].tag, &sHTMLMarkupMapList[i]);
>  
>  #ifdef MOZ_XUL
> -  for (uint32_t i = 0; i < ArrayLength(sXULMapList); i++)
> +  for (uint32_t i = 0; i < ArrayLength(sXULMarkupMapList); i++)

Warning: Use range-based for loop instead [clang-tidy: modernize-loop-convert]

Comment 10

[Adrian Wielgosik [:adrian17]]

Comment on attachment 8948395 [details]
Bug 1414230 - Part 2 - Simplify XUL markup map creation.

https://reviewboard.mozilla.org/r/217870/#review223634

::: accessible/base/nsAccessibilityService.h:74
(Diff revision 1)
>  };
>  
>  #ifdef MOZ_XUL
>  struct XULMarkupMapInfo {
>    nsStaticAtom** tag;
> -  New_Accessible* new_func;
> +  std::function<Accessible*(nsIContent* aContent,

Since the new lambdas are stateless, they are convertible to function pointers, so using `New_Accessible*` here should still work.

Comment 11

[alexander :surkov (:asurkov)]

Comment on attachment 8948394 [details]
Bug 1414230 - Part 1 - Align XUL and HTML markup table names.

https://reviewboard.mozilla.org/r/217868/#review223676

Comment 12

[alexander :surkov (:asurkov)]

Comment on attachment 8948395 [details]
Bug 1414230 - Part 2 - Simplify XUL markup map creation.

https://reviewboard.mozilla.org/r/217870/#review223702

looks good with me

Comment 13

[:Paolo Amadini]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6f41dea48faaebb4299dc2fd5ce60e9b932dcd2

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4ed6acdfc6b
Part 1 - Align XUL and HTML markup table names. r=surkov
https://hg.mozilla.org/integration/mozilla-inbound/rev/2de998ec30b0
Part 2 - Simplify XUL markup map creation. r=surkov

Comment 15

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/d4ed6acdfc6b
https://hg.mozilla.org/mozilla-central/rev/2de998ec30b0