[Bug 948788]
remote: https://hg.mozilla.org/integration/mozilla- inbound/rev/a5aa0a654611 -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
Comment on attachment 738132 Implement a replacement of atk_object_set_name() which mimics the behavior without calling atk_object_get_name() landed again, but forgot to qref -u sorry :( -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
remote: https://hg.mozilla.org/integration/mozilla- inbound/rev/ecd327272240 -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
Comment on attachment 729066 Implement a replacement of atk_object_set_name() which mimics the behavior without calling atk_object_get_name() sorry it took me so long, but I finally got to test your patch today. It seems good so I checked it in for you ask you just saw :-) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
Comment on attachment 728261
Implement a replacement of atk_object_set_name() which mimics the behavior
without calling atk_object_get_name()
diff --git i/accessible/src/atk/AccessibleWrap.cpp
w/accessible/src/atk/AccessibleWrap.cpp
index e35da5d..208e955 100644
--- i/accessible/src/atk/AccessibleWrap.cpp
+++ w/accessible/src/atk/AccessibleWrap.cpp
@@ -142,16 +142,20 @@ struct MaiAtkObjectClass
static guint mai_atk_object_signals [LAST_SIGNAL] = { 0, };
#ifdef MAI_LOGGING
int32_t sMaiAtkObjCreated = 0;
int32_t sMaiAtkObjDeleted = 0;
#endif
G_BEGIN_DECLS
+
+static void
+MaybeFireNameChange(AtkObject *aAtkObj, const nsAutoString aNewNameUTF16);
there's no reason this needs to be extern C which is all the G_DECL
thing is hiding is there?
btw type* name
nsAutoString uniName;
accWrap-Name(uniName);
- NS_ConvertUTF8toUTF16 objName(aAtkObj-name);
- if (!uniName.Equals(objName))
-atk_object_set_name(aAtkObj, NS_ConvertUTF16toUTF8(uniName).get());
+ // XXX Firing an event from here does not seem right
+ MaybeFireNameChange(aAtkObj, uniName);
nit, might be nice if you renamed the var to just name since we don't
convert it here
+MaybeFireNameChange(AtkObject* aAtkObj, const nsAutoString
aNewNameUTF16)
nit, utf16 is implied by the type being nsFooString not nsFooCString so
kind of redundant
+{
+ NS_ConvertUTF8toUTF16 curNameUTF16(aAtkObj-name);
wouldn't it be more efficient to just convert the new name to utf8?
+
+ if (aNewNameUTF16.Equals(curNameUTF16)) {
+return;
+ }
nit, no braces
+ const gchar* name = NS_ConvertUTF16toUTF8(aNewNameUTF16).get();
so, that creates an object that only lives for the statement which is
going to mean accessing atkObj-name after this statement is a use after
free.
if you convert newName to utf8 at the start you can just do
free(atkOjb-name); atkObj-name = strdup(newNameUTF8.get());
+
+ // Below we duplicate the functionality of atk_object_set_name(),
+ // but without calling atk_object_get_name(). Instead of
+ // atk_object_get_name() we directly access aAtkObj-name. This is because
+ // atk_object_get_name() would call getNameCB() which would call
+ // MaybeFireNameChange() (or atk_object_set_name() before this problem was
+ // fixed) and we would get an infinite recursion.
+ // See http://bugzilla.mozilla.org/733712
+
+ AtkObjectClass *klass;
+ gboolean notify = FALSE;
+
+ g_return_if_fail(ATK_IS_OBJECT(aAtkObj));
+ g_return_if_fail(name != NULL);
+
+ klass = ATK_OBJECT_GET_CLASS(aAtkObj);
+ if (klass-set_name) {
+// Do not notify for initial name setting.
+// See bug http://bugzilla.gnome.org/665870
+notify = (aAtkObj-name != NULL);
+
+(klass-set_name)(aAtkObj, name);
+if (notify) {
+ g_object_notify(G_OBJECT(aAtkObj), atk_object_name_property_name);
+}
+ }
the only part of this you need is the if notify g_object_notify() bit
unless I'm missing something
thanks for helping to clean this up it looks good other than that.
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/948788
Title:
thunderbird crashed on launch
To manage notifications about this bug go to:
https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
(In reply to alexander :surkov from comment #39) (In reply to Trevor Saunders (:tbsaunde) from comment #38) (In reply to alexander :surkov from comment #37) atk_object_set_name() called inside get_name is a wrong thing we try to I think the only thing we try to fix here is the infinite recurssion. and code madnness :) why? the reason for the bug was crashes with a change to atk, so just fixing the way we interact with atk should be fine. remove here. It made us fire name change events which is ridiculous I think. If I get right then ATK implementation internals don't need that event as long as we override get_name. On the another hand I don't understand why the I believe it needs event to keep cache in sync with reality. consumer might need name change event when it asks for the name. what if there is more than one consumer, then it may be arguably less bad for other consumers to get the event late rather than never. I absolutely agree what we do is crazy, but I know there is caching involved and I'm atleast somewhat concerned not fireing an event in getName() could break that even more than it is now. what kind of caching? And how does this caching is supposed to work if somebody asks us to calculate the name (bypassing that cache)? Or alternatively who uses that cache and why all consumers don't want to use it? I think the way it works is that consumer processes have a local repreentation of atkobject in their process which keeps the name and atk updates the name based on name change event. So each consumer can choose for itself to use or not use cache as it likes, and might well want to not use the cache because it can easily become out of date due to us not always firing name change events. If your offering to fix name change events so they're fired whenever a name changes and never when it doesn't then of course I'm happy to remove this madness. iirc AT needs this event every time when name is changed. In this sense our name change event might never work for this purpose. its possible in any case I'm not really willing to remove the madness we have now atleast until we talk to atk people about it, and I don't see a reason to make vd block on that. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
Comment on attachment 726580
Implement a replacement of atk_object_set_name() which mimics the behavior
without calling atk_object_get_name()
+static void
+AtkObjectSetName(AtkObject *aAtkObj, const gchar *name)
so, both of the times this is called we have to check if we actually
want to fire an event first. So why don't we refactor this stuff like
this
make this function
static void
MaybeFireNameChange(AtkObject* aObj, nsString aNewName)
then it can incapsilate the stuff about comparing against the old name.
+size_t name_len = strlen(name);
+
+if (strlen(aAtkObj-name) = name_len) {
+ /* If the new name is shorter, then just use the old memory chunk
+ * to minimize memory fragmentation. */
+ memcpy(aAtkObj-name, name, name_len + 1);
+} else {
+ g_free(aAtkObj-name);
+ aAtkObj-name = g_strdup(name);
I'd be suprised if this isn't a win and possibly a loss. Especially
since the best you can do is strlen while the allocator actually knows
how big the block is.
otherwise this seems like the correct approach so f=me but I'd like to
see another version before r+ :)
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/948788
Title:
thunderbird crashed on launch
To manage notifications about this bug go to:
https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
(In reply to alexander :surkov from comment #35) (In reply to Trevor Saunders (:tbsaunde) from comment #34) Comment on attachment 726580 Implement a replacement of atk_object_set_name() which mimics the behavior without calling atk_object_get_name() +static void +AtkObjectSetName(AtkObject *aAtkObj, const gchar *name) so, both of the times this is called we have to check if we actually want to fire an event first. we don't want an event in the former case not sure what you mean. both places we call atk_object_set_name() we guard it with something that is more or less oldName != newName now, the first of those shouldn't be needed in an ideal world because we'd always fire name change event, but we don't do that because its between hard and impossible to do that because name algorithm is complicated and depends on all sorts of stuff. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
(In reply to alexander :surkov from comment #37) atk_object_set_name() called inside get_name is a wrong thing we try to I think the only thing we try to fix here is the infinite recurssion. remove here. It made us fire name change events which is ridiculous I think. If I get right then ATK implementation internals don't need that event as long as we override get_name. On the another hand I don't understand why the consumer might need name change event when it asks for the name. I absolutely agree what we do is crazy, but I know there is caching involved and I'm atleast somewhat concerned not fireing an event in getName() could break that even more than it is now. If your offering to fix name change events so they're fired whenever a name changes and never when it doesn't then of course I'm happy to remove this madness. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
Comment on attachment 725350 Implement setNameCB() in AccessibleWrap.cpp and use it in getNameCB() instead of atk_object_set_name() @@ -644,11 +644,40 @@ getNameCB(AtkObject* aAtkObj) NS_ConvertUTF8toUTF16 objName(aAtkObj-name); if (!uniName.Equals(objName)) -atk_object_set_name(aAtkObj, NS_ConvertUTF16toUTF8(uniName).get()); +setNameCB(aAtkObj, NS_ConvertUTF16toUTF8(uniName).get()); you don't need to implement SetNameCB() or deal with atkobj-name at all you should be able to just do g_object_notify(blah); @@ -1011,6 +1040,7 @@ AccessibleWrap::FirePlatformEvent(AccEvent* aEvent) accessible-Name(newName); NS_ConvertUTF16toUTF8 utf8Name(newName); if (!atkObj-name || !utf8Name.Equals(atkObj-name)) + /* XXX also use setNameCB() here? */ atk_object_set_name(atkObj, utf8Name.get()); just change here too. Note we should do the same thing for atk_object_set_parent() and atk_object_set_description() btw I don't really care but most people around here like 8 lines of context in diffs. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
(In reply to alexander :surkov from comment #25) (In reply to JeffreyDecker from comment #24) I would like to take on this bug if that's alright. done, thank you btw. Just reading through the comments though it looks like it's a little unclear as to how we want to refactor to fix the bug. I think go with solution from comment #21. Has there been any decision made? Ginn, Trevor, does comment #21 approach sounds acceptable for you? seems fine. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 948788]
(In reply to alexander :surkov from comment #21) (In reply to Ginn Chen from comment #20) We need to copy the string to gchar* anyway. yep, just copy the string every time when we were asked. Doesn't sound good? Actually it's not possible. atk_object_get_name() returns const gchar*, caller will not free the return value. We have to keep it somewhere in AtkObject, otherwise we leak. I see So if provide getName/setName implementation that will use accessible-name to store a value then would it be acceptable solution? or we could do what we do for a bunch of other atk methods reutrning const gchar* nd use nsAccessibleWrap::ReturnString() which keeps a static nsCString and returns a pointer into its buffer, and so requires caller of atk method to copy returned string before calling another atk function, which I think its very likely the bridge will always do. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/948788 Title: thunderbird crashed on launch To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/948788/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 857153]
landed https://hg.mozilla.org/releases/mozilla-aurora/rev/c0df4333b5b7 -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/857153 Title: Needs to get accessibility settings from GSettings To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/857153/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 857153]
[Approval Request Comment] Regression caused by (bug #): no bug, gnome's move away from gconf Testing completed (on m-c, etc.): Risk to taking this patch (and alternatives if risky): The old behavior of checking gconf will be used if the dbus call does not succeed (ie, if AT-SPI2 is not installed), so I think that the risk is small. The it is theoretically possible this could somehow cause a11y to be enabled by accident which would cause a perf regression, however I would evaluate the probaility of this as low, and if it was a problem I would expect it to be a fairly general problem which we should have already heard about. An alternative approach would be to modify the start-up script to call dbus-send and set GNOME_ACCESSIBILITY=1 if the call indicates that accessibility is enabled, but this would likely affect start-up time more than applying the patch would, so applying the patch seems preferable. The shell script went away, so this might not be a possibility, and it would be completely new untested code. The regression is pretty significant (we've seen mails from people wondering why firefox is no longer accessible), and since Fire Fox 10 will be an esr and so around for a while to some degree, I'll do my best to take Surkov's place (he's out) and say I think we want to take this. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/857153 Title: Needs to get accessibility settings from GSettings To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/857153/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 857153]
https://hg.mozilla.org/mozilla-central/rev/e95cb5b98154 https://hg.mozilla.org/mozilla-central/rev/39acd9b60ebc -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/857153 Title: Needs to get accessibility settings from GSettings To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/857153/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 857153]
I wonder whether the PreInit() in ShouldA11yBeEnabled() is really necessary
or
could be replaced with an assertion to check that PreInit had already been
called. Perhaps it is needed for unit tests?
I don't think I'm familiar enough with when things happen in widget/ to
be sure that call to PreInit() will come first.
-if (aState sAccessibilityEnabled) {
+if (aState a11y::ShouldA11yBeEnabled())
CreateRootAccessible();
-}
Please don't unbrace the block here. Convention in this file is heading
towards always brace except for jump statements, but usually leave existing
code as is.
ok :(
What does CreateRootAccessible() actually achieve? It looks like
mRootAccessible is unused. Is it holding a reference so that
DispatchEventToRootAccessible will somehow not need to create another
nsAccessible?
well, msot of that code came before me, but the only current purpose I
can think of is to not fire the event if we don't need to since the root
accessible is already created. Of course using a ref to the root
accessible seems a silly way to do that, maybe Ginn or Alex knows more.
I suspect a spin off bug to clean up thi would make sense, I'll gues
this code hasn't been worked on in a good while for the most part.
I suppose its fairly clear though that the main purpose is to cause a11y to
create a root accessible.
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/857153
Title:
Needs to get accessibility settings from GSettings
To manage notifications about this bug go to:
https://bugs.launchpad.net/firefox/+bug/857153/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 857153]
(In reply to Karl Tomlinson (:karlt) from comment #47) (In reply to Karl Tomlinson (:karlt) from comment #44) If there is no session, then I assume there will be no org.a11y.Bus. Actually there probably can still be a org.a11y.Bus if registered in /usr/share/dbus-1/services/ http://www.freedesktop.org/wiki/IntroductionToDBus#Activation But it looks like the problem is resolved, so perhaps this workaround can be used until a better solution is found. well, the firstthing that comes to mind is arranging for build machines and test machines to have GNOME_ACCESSIBILITY set, do we have an idea how hard this is? I'd expect though that in the vast majority of cases that there is a session and this is not relavent. Karl would you object if I merge that change set? (with nits if you have any fixed), I should atleast look at the length of the check I added since it may well be over 80 chars. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/857153 Title: Needs to get accessibility settings from GSettings To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/857153/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 857153]
(In reply to Karl Tomlinson (:karlt) from comment #50) (In reply to Trevor Saunders (:tbsaunde) from comment #49) well, the firstthing that comes to mind is arranging for build machines and test machines to have GNOME_ACCESSIBILITY set, do we have an idea how hard this is? It would be good to ensure there is a session bus, because we'd like to test the dbus code. Perhaps DBUS_SESSION_BUS_ADDRESS is not set because of the We don't have any tests right now that make sure that this code works properly afaik, or for that matter any of our platform code works. way our testing connects to the existing gnome session. Or perhaps the CentOS 5.0 machines are just too old to have a session bus running. On Is there even a gnome session to speak of? If you only need accessibility enabled for accessibility tests then it probably makes sense to set GNOME_ACCESSIBILITY in the Makefile command that runs the accessiblity tests. I believe the current state to be that accessibility is only on for a11y tests where it is turned on through xpcom instead of using platform mechanisms I'd expect though that in the vast majority of cases that there is a session and this is not relavent. Karl would you object if I merge that change set? No I would not object. Please add a comment to explain why the DBUS_SESSION_BUS_ADDRESS environment variable is there though. sure The situation with dbus when it can't connect to a session bus is unfortunate, and perhaps something worth avoiding anyway, if we can. yeah, I'd expect we'd spend a bit of time blocking though that's a guess. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/857153 Title: Needs to get accessibility settings from GSettings To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/857153/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 857153]
(In reply to Karl Tomlinson (:karlt) from comment #44) That was really just guessing in comment 42, but would it make sense to skip the check if DBUS_SESSION_BUS_ADDRESS is unset anyway? If there is no session, then I assume there will be no org.a11y.Bus. I was thinking the same thing, but wasn't sure if that variable was required to have a useful dbus session. Since I haven't reproduced this locally and it seemed worth a shot I pushed https://hg.mozilla.org/projects/accessibility/rev/e95cb5b98154 lets see how it does. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/857153 Title: Needs to get accessibility settings from GSettings To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/857153/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 857153]
(In reply to Reed Loden [:reed] (very busy) from comment #3) probably karlt for the gtk2 widget stuff, and maybe surkov or dbolter for the a11y stuff... or maybe just karlt alone. actually, me or Ginn Chen are probably the better a11y people here, and I think one of us should look at that sort of thing. That said we're already working on this in bug 693343, if you'd like to help figure out the shutdown hang were having trouble with there that would be great! *** This bug has been marked as a duplicate of bug 693343 *** -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/857153 Title: Needs to get accessibility settings from GSettings To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/857153/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
