If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

create new folder for util files

RESOLVED WORKSFORME

Status

()

Core
Disability Access APIs
RESOLVED WORKSFORME
9 years ago
5 years ago

People

(Reporter: surkov, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [bk1])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

9 years ago
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.
(Reporter)

Comment 1

8 years ago
Created attachment 421133 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #421133 - Flags: review?(bolterbugz)
(Reporter)

Comment 2

8 years ago
try-server build - http://build.mozilla.org/tryserver-builds/surkov.alexander@gmail.com-try-d4e0d6cb38d7, it's not necessary but it is :)
(Reporter)

Comment 3

8 years ago
Created attachment 421224 [details] [diff] [review]
patch

correct version
Attachment #421133 - Attachment is obsolete: true
Attachment #421224 - Flags: review?(bolterbugz)
Attachment #421133 - Flags: review?(bolterbugz)
This looks okay, I'm just leary of this landing because there are some large patches in my queue (which will bitrot).
(Reporter)

Comment 5

8 years ago
(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.
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)
(Reporter)

Comment 7

8 years ago
(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?
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?
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?
(Reporter)

Comment 10

8 years ago
(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.
(Reporter)

Comment 11

8 years ago
Created attachment 421322 [details] [diff] [review]
patch3
Attachment #421224 - Attachment is obsolete: true
Attachment #421322 - Flags: review?(bolterbugz)
Attachment #421224 - Flags: review?(bolterbugz)
I'm not comfortable with the name "generic" for a directory that contains helpers and utils.
(Reporter)

Comment 13

8 years ago
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.
(Reporter)

Comment 14

8 years ago
(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.
(In reply to comment #13)
> Let's ask Ginn.
Yep. (cc'ed)

Comment 16

8 years ago
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.
(Reporter)

Comment 17

8 years ago
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 on attachment 421322 [details] [diff] [review]
patch3

Removing myself from request queue for now.
Attachment #421322 - Flags: review?(bolterbugz)
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)?
Whiteboard: [bk1]
(Reporter)

Comment 20

6 years ago
(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.
(Reporter)

Comment 21

5 years ago
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
Assignee: surkov.alexander → nobody
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.