The extracted method can at least be used on Mac (see bug 718700).

making bug a more generic, xml-roles is not unique case we have (for example, ongoing IA2 1.3 needs it)

changing to Accessible::Attribute(const nsIAtom* aName, nsAString& aValue) as Trev suggested iirc

It seems it'd be a good idea if we had enum for attribute names, we could do a switch which faster than atom ifing. Ok?

(In reply to alexander :surkov from comment #3) > It seems it'd be a good idea if we had enum for attribute names, we could do > a switch which faster than atom ifing. Ok? I'd guess it'll be slightly faster since the enum values are compile time constants, and never 8 bytes as pointers are on x64, but afaik the real win with a switch is using a jump table instead of a large number of compare and jumps which I suspect isn't going to be possible here since I think all the switches will look like switch (value) { case 5: x; break; case 1: y; break; default: supper::foo(); } not switch (value) { case 0: x; break; case 1: y; break; case 2: z; break; ... in order case 20: c; break; default: return 0; }

(In reply to Trevor Saunders (:tbsaunde) from comment #4) > switch (value) { > case 5: x; break; > case 1: y; break; > default: supper::foo(); > } making them alphabetical we can do switch (value) { case 1: x; break; case 5: y; break; default: supper::foo(); } so compilers don't do any maric for this?

(In reply to alexander :surkov from comment #5) > (In reply to Trevor Saunders (:tbsaunde) from comment #4) > > > switch (value) { > > case 5: x; break; > > case 1: y; break; > > default: supper::foo(); > > } > > making them alphabetical we can do > > switch (value) { > case 1: x; break; > case 5: y; break; > default: supper::foo(); > } > > so compilers don't do any maric for this? I don't think there's much you *can* do for something like that other than compare against each posibility and branch. On the other hand if you have all the numbers in some range you can just check your within the range and then compute an address based on where in the range you are to jump to.

generally the biggest if belongs to Accessible version, other versions usually don't expose more than couple of attributes. So if would do an enum then we are targeted primarily to Accessible::NativeAttributes and perhaps that's ok. Alternatively we could provide split this method into a number of methods like XMLRolesAttr or just XMLRoles(). Not sure it makes things nicer.

(In reply to alexander :surkov from comment #7) > generally the biggest if belongs to Accessible version, other versions > usually don't expose more than couple of attributes. So if would do an enum > then we are targeted primarily to Accessible::NativeAttributes and perhaps > that's ok. > > Alternatively we could provide split this method into a number of methods > like XMLRolesAttr or just XMLRoles(). Not sure it makes things nicer. I think I like enum or atom approach better, but don'tcare much between them.

So, I don't totally like the retun a string approach since it can often mean we need to turn int / bool / atom into a string, and then in the case of some platforms turn that string back into a useful value. SO some alternate API ideas struct AttrValue { enum Type { eAtom, eString, eInt, eBool, eUint }; Type mType; uintptr_t value; operator =(int aVal) { mType = eInt; mValue = aVal; } operator =(bool aVal) { mType = eBool; mValue = aVal; } Type Type() const { return mType; } operator int() { NS_ASSERTION(mType == eInt, "converting non int value to int!"); return static_cast<int>(mValue); } operator nsIAtom*() { NS_ASSERTION(mType == eAtom, "turning not atom value into a atom!!!"); return static_cast<nsIAtom*>(mValue); } then we'd have AttrValue Attribute(nsIAtom* aAttr); strings might be a little tricky since there larger than a pointer, but that can probably be sorted out. or we could do each platform defines a class / inherits from a generic one class ValueCallback { operator =(nsIAtom* aVal) { mHashtable.put(mAttr, aVal); } or for different platform operator =(bool aVal) { mString.Append(":"); mString.Append(aVal ? "true" : "false"); } }; then have method void Attribute(nsIAtom* aAttr, ValueCallback& aVal) { if (aAttr == nsGkAtoms::level) { aVal = 5; return; } thoughts?

1) Is uintptr_t preferable over union? 2) We could avoid string usage by dealing with nsIAtom*. It makes sense if HTML attributes (ARIA attributes) are atom based. 3) Not sure I see connection between AttrValue and ValueCallback&. On internal level it should be nice to have: AttrValue Accessible::Attribute(nsIAtom* aName); I think I lean towards to have an enum for attribute names, it should be a little bit faster than if else (atom ==)

(In reply to alexander :surkov from comment #10) > 1) Is uintptr_t preferable over union? well, it ensures you won't put anything bigger than a pointer in it, but I'm not sure which is more reasonable here. > 2) We could avoid string usage by dealing with nsIAtom*. It makes sense if > HTML attributes (ARIA attributes) are atom based. I think we can most of the time, just not sure about all of it. > 3) Not sure I see connection between AttrValue and ValueCallback&. they're two different approaches. > I think I lean towards to have an enum for attribute names, it should be a > little bit faster than if else (atom ==) maybe slightly, I don't particularly care, but I tend to lead away from one off enums that are subsets of similar things.

(In reply to Trevor Saunders (:tbsaunde) from comment #11) > (In reply to alexander :surkov from comment #10) > > 1) Is uintptr_t preferable over union? > > well, it ensures you won't put anything bigger than a pointer in it, but I'm > not sure which is more reasonable here. union might be nicer > > 2) We could avoid string usage by dealing with nsIAtom*. It makes sense if > > HTML attributes (ARIA attributes) are atom based. > > I think we can most of the time, just not sure about all of it. > > > 3) Not sure I see connection between AttrValue and ValueCallback&. > > they're two different approaches. > > > I think I lean towards to have an enum for attribute names, it should be a > > little bit faster than if else (atom ==) > > maybe slightly, I don't particularly care, but I tend to lead away from one > off enums that are subsets of similar things. ok would you like to take it?

in the light of innerHTML object attribute how would AttrValue approach look like?

(In reply to alexander :surkov from comment #13) > in the light of innerHTML object attribute how would AttrValue approach look > like? what is the question? innerHTML is just a big string type value.

atoms idea doesn't work so you said: then we'd have AttrValue Attribute(nsIAtom* aAttr); strings might be a little tricky since there larger than a pointer, but that can probably be sorted out. the question is how

(In reply to alexander :surkov from comment #15) > atoms idea doesn't work so you said: use raw nsStringBuffer*, or maybe nsString*?

short example pls? who will own the string?