481361 - create new folder for util files

Status: RESOLVED WORKSFORME

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Currently all our util and helper files are hosted in base directory within base a11y classes. Last time we get more and more util and helper files so it's time to think about separate directory for them. I would suggest to create "general" directory and move them there.

In the meantime I suggest to move 

nsCoreUtils, nsAccUtils, nsAccessibilityAtoms, nsAccessibleTreeWalker, nsARIAMap, nsRelUtils, nsTextAttrs, nsTextEquivUtils - all we do not expose anyhow to AT.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[alexander :surkov (:asurkov)]

try-server build - http://build.mozilla.org/tryserver-builds/[e-mail]-try-d4e0d6cb38d7, it's not necessary but it is :)

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch

correct version

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

This looks okay, I'm just leary of this landing because there are some large patches in my queue (which will bitrot).

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #4)
> This looks okay, I'm just leary of this landing because there are some large
> patches in my queue (which will bitrot).

I don't mind if you want to put some patches before this one. Let's mark your bug as blocking this one. Also I hope you do not need much time to land your patches because I don't want this bug to hang forever.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Maybe best not to wait, but I'm unsure about "generic" as it is an unusual directory name and makes me think of "generics". What about making a subdirectory of base called "util" or "helpers"? Not sure I like that either.

(Note: https://developer.mozilla.org/en/Source_code_directories_overview)

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> Maybe best not to wait, but I'm unsure about "generic" as it is an unusual
> directory name and makes me think of "generics". What about making a
> subdirectory of base called "util" or "helpers"? Not sure I like that either.

I don't have strong opinion. I denied "util" because nsAccessibleTreeWalker and nsAccessibilityService is not util, "helper" because "nsAccessibilityService" is not a helper, it's primary thing. I was inspired by layout module where "generic" directory is presented. However it sounds we should change the content of our "base" and "generic" directories to correspond the rules. So that "base" would be that cannot be categorized into a submodule, "generic" would contain base classes. Is it better?

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

Okay, I took a look to see what layout does. I'm not sure what the distinction between layout/base and layout/generic is... Roc?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

layout/generic is the set of "generic" frame implementations for block-and-line layout and other frame types that don't belong anywhere else --- plugins, scrolling, columns etc. layout/base is the core layout code that's not specific to any particular frame type.

Personally I don't see a need for you to split out accessibility code here. Are you worried that helper code doesn't logically belong with the base code, or are you worried about too many files in one directory?

Comment 10

[alexander :surkov (:asurkov)]

(In reply to comment #9)

> Personally I don't see a need for you to split out accessibility code here. Are
> you worried that helper code doesn't logically belong with the base code, or
> are you worried about too many files in one directory?

Both. Accessible object base classes are mixed with until and helper classes. It's not very nice and it's too lot of files and their number will grow I think.

Comment 11

[alexander :surkov (:asurkov)]

Attachment: patch3

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

I'm not comfortable with the name "generic" for a directory that contains helpers and utils.

Comment 13

[alexander :surkov (:asurkov)]

Let's ask Ginn.

base is for utils, tree walker, accessibility service
generic is for base and common c++ classes like nsAccessNode, nsAccessible,
nsDocAccessible, nsAccEvent and etc.

Comment 14

[alexander :surkov (:asurkov)]

(In reply to comment #12)
> I'm not comfortable with the name "generic" for a directory that contains
> helpers and utils.

"generic" is for nsAccessNode and etc, "base" is for helpers and utils.

Comment 15

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #13)
> Let's ask Ginn.
Yep. (cc'ed)

Comment 16

[Ginn Chen]

I don't see a strong need to split the directory as we don't have too much files.

If we want to do it, comment 13 sounds reasonable.

Comment 17

[alexander :surkov (:asurkov)]

Ok, personally I like the new folder structure more than existing one. But if you all are kind of skeptic about this then we could get back to this later when/if we get more files in the "base" directory.

Comment 18

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 421322 [details] [diff] [review]
patch3

Removing myself from request queue for now.

Comment 19

[David Bolter [:davidb] (NeedInfo me for attention)]

Alexander in the interest of reducing bug count noise can we close this bug and do file organization on a separate bug (or create a bug when we're ready to do this work)?

Comment 20

[alexander :surkov (:asurkov)]

(In reply to David Bolter [:davidb] from comment #19)
> Alexander in the interest of reducing bug count noise can we close this bug
> and do file organization on a separate bug (or create a bug when we're ready
> to do this work)?

that could be ok, but it might make sense to keep this bug to do incremental changes.

Comment 21

[alexander :surkov (:asurkov)]

this bug was fixed as it was suggested in comment #13. I don't want to search the history so marking the bug as worksforme