Closed
Bug 290352
Opened 20 years ago
Closed 20 years ago
Implement accessible DHTML sub-sub menus
Categories
(Core :: Disability Access APIs, defect, P2)
Tracking
()
RESOLVED
FIXED
People
(Reporter: aaronlev, Assigned: aaronlev)
References
Details
(Keywords: access)
Attachments
(3 files, 8 obsolete files)
75.60 KB,
patch
|
Louie.Zhao
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
2.67 KB,
text/html
|
Details | |
75.17 KB,
patch
|
bzbarsky
:
superreview+
asa
:
approval1.8b3+
|
Details | Diff | Splinter Review |
We have accessible DHTML menus, need to make sub-sub menus work.
See www.mozilla.org/access/samples/dhtml-spreadsheet.html for example of menubar.
We need a testcase that has sub-sub menus to get started with this.
Assignee | ||
Updated•20 years ago
|
Priority: -- → P2
Assignee | ||
Comment 1•20 years ago
|
||
This worked out of the box with the sample page Becky Gibson created:
http://www.mozilla.org/access/dhtml/spreadsheet
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → WORKSFORME
Assignee | ||
Comment 2•20 years ago
|
||
It doesn't currently fire the right menupopupend events.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Assignee | ||
Comment 3•20 years ago
|
||
This fixes a few problems:
1. Sub-menus and Sub-sub menus now get symmetrical popupstart/popupend events
at the right times, with less code
2. DHTML accessibility alert events (bug 289902)
3. Show/hide are fired on the child that changes, and reorder is fired on the
container who's children changed. This is desirable, compared with what we had
before.
4. Double mutation events for same item are no longer received -- this was
occuring because we were listening on XUL and HTML doc.
Assignee | ||
Updated•20 years ago
|
Attachment #183087 -
Flags: superreview?(bzbarsky)
Attachment #183087 -
Flags: review?(bzbarsky)
![]() |
||
Comment 4•20 years ago
|
||
The patch is wrong if visibility changes from hidden to visible and at the same
time display changes from block to none, say. I'll report an EVENT_SHOW in that
case, which is not really desirable, imo.
Past that, I'm not completely sure with putting this in the style system in
general (if nothing else it breaks the current symmetry of the CalcDifference
functions on style structs).
Assignee | ||
Comment 5•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #183087 -
Flags: superreview?(bzbarsky)
Attachment #183087 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 6•20 years ago
|
||
David, can you r= just the layout parts please? Bz wants to know what you think
of the accessibility change hints that I needed, and potential perf impact.
Thank you.
Attachment #183117 -
Flags: superreview?(bzbarsky)
Attachment #183117 -
Flags: review?(dbaron)
So, the approach here seems counter to the entire design of the CalcDifference
methods. They're designed to say "invalidate your knowledge regarding this
aspect of this subtree". They're not designed to say how it changed, or whether
the entire subtree changed or just part of it, and we optimize away many
notifications on descendants for that reason.
This means that there are a whole bunch of cases where you won't get the
notifications you expect: when an element changes visibility and a descendant
changes the other way you'll get both notifications, no matter how they're
changing (or how any descendants are also changing). (Right? Or did you hack
this even more than I thought from a quick glance?)
That said, I'd question even whether the 'visibility' property should be
reflected into aural rendering at all. It's supposed to be used for visual
media only.
![]() |
||
Comment 8•20 years ago
|
||
Comment on attachment 183117 [details] [diff] [review]
Address bz's edge case (see test case)
David is right; this is wrong in a whole bunch of cases.
Attachment #183117 -
Flags: superreview?(bzbarsky)
Attachment #183117 -
Flags: superreview-
Attachment #183117 -
Flags: review?(dbaron)
Attachment #183117 -
Flags: review-
Assignee | ||
Comment 9•20 years ago
|
||
> That said, I'd question even whether the 'visibility' property should be
> reflected into aural rendering at all. It's supposed to be used for visual
> media only.
It's important to the screen reader vendors and their users that they get an
accurate representation of the visual display of a web page. If we someday get
to the world of voice browsers that use aural CSS, that's fine -- but it's not
what the screen reader user model is all about. Screen readers show the blind
user what is happening on the screen visually -- hence our use of layout info in
general.
That said, do you have another recommendation of how to approach this so I can
get notification of things becoming visible/invisible?
Assignee | ||
Comment 10•20 years ago
|
||
Is the CSS display approach we took approach in RecreateFramesForContent
alright? It doesn't use the change hints -- it just checks to see if a frame is
being created or going away for that content.
> when an element changes visibility and a descendant
> changes the other way you'll get both notifications, no matter how they're
> changing (or how any descendants are also changing).
In this case we're okay. Because the accessibility module gets notified we
invalidate the tree, and it will be created as needed. Nothing that shouldn't be
created will, etc. Without any notification at all the accessibility module will
just assume things are as they wre.
![]() |
||
Comment 11•20 years ago
|
||
I think David's point was that with your patch you could get a, say, HIDE
invalidate on an element while not all the kids of the element are getting hidden.
Assignee | ||
Comment 12•20 years ago
|
||
(In reply to comment #11)
> I think David's point was that with your patch you could get a, say, HIDE
> invalidate on an element while not all the kids of the element are getting hidden.
That's really okay -- I can and do check still look at the dom/layout trees
after the notification. The only thing it would do wrong right now based on this
description, is send the screen reader an EVENT_HIDE or EVENT_SHOW on an
ancestor instead of the descendent. However, the accessibility tree would still
get properly invalidated and updated.
The only real problem is when the accessibility module does not get notified of
important changes.
![]() |
||
Comment 13•20 years ago
|
||
But I thought that we _were_ already invalidating (that's the
nsChangeHint_AccessibleChange, and that part is fine). What this patch is doing
is trying to send the SHOW/HIDE events, no?
Assignee | ||
Comment 14•20 years ago
|
||
(In reply to comment #13)
> But I thought that we _were_ already invalidating (that's the
> nsChangeHint_AccessibleChange, and that part is fine). What this patch is doing
> is trying to send the SHOW/HIDE events, no?
Okay, I'll look into what happens when I leave that the same and calculate
show/hide by looking at the frame tree in the accessibility module.
Does the RecreateFramesForContent() change need to go as well?
Assignee | ||
Comment 15•20 years ago
|
||
Bz, I'd like to also keep the change where we pass in the extra parameter to
InvalidateSubtreeFor() so that we get the parentContent and content for the
nsChangeHint_AccessibleChange. Is that sensible?
![]() |
||
Comment 16•20 years ago
|
||
Yeah, that should be ok....
Assignee | ||
Comment 17•20 years ago
|
||
Attachment #183087 -
Attachment is obsolete: true
Attachment #183117 -
Attachment is obsolete: true
Attachment #183915 -
Flags: superreview?(bzbarsky)
Attachment #183915 -
Flags: review?(bzbarsky)
![]() |
||
Comment 18•20 years ago
|
||
It'll be at least a week before I can get to this.
Assignee | ||
Updated•20 years ago
|
Attachment #183915 -
Attachment is obsolete: true
Attachment #183915 -
Flags: superreview?(bzbarsky)
Attachment #183915 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 19•20 years ago
|
||
Bz, thanks for the heads up on the week before you can review.
Attachment #183926 -
Flags: superreview?(bzbarsky)
Attachment #183926 -
Flags: review?(bzbarsky)
![]() |
||
Comment 20•20 years ago
|
||
Comment on attachment 183926 [details] [diff] [review]
One more Eureka :) For style changes, InvalidateSubtree() from my nsIDocumentObserver style change notifications intead of from nsPresShell::ReconstructStyleDataInternal() which was too early
>Index: layout/base/nsCSSFrameConstructor.h
>+ nsIAtom *GetRenderedFrameType(nsIFrame *aFrame);
>+ void NotifyAccessibleChange(nsIAtom *aPreviousFrameType, nsIAtom *aFrameType,
Document what these do, please.
>Index: layout/base/nsCSSFrameConstructor.cpp
>+nsCSSFrameConstructor::GetRenderedFrameType(nsIFrame *aFrame)
>+ if (vis->mVisible == NS_STYLE_VISIBILITY_VISIBLE) {
Why not use vis->IsVisible() here? That'll be forward-compatible to when other
visibility values are added and all...
nsCSSFrameConstructor::ProcessRestyledFrames(nsStyleChangeList& aChangeList)
> continue;
>+
> }
Take that out, please.
>@@ -11596,45 +11635,23 @@ nsCSSFrameConstructor::RecreateFramesFor
Why the logic changes here? Is there a good reason for those? If not, please
take them out.
Note that reframes can take place in the absence of style changes. See
nsIPresShell::RecreateFramesFor, which anyone who feels like it can call.
As written, the changes in this patch will miss some show/hide events (eg if
visibility of both a parent and a child changes we can dispatch a hide on the
parent while the child remains visible).
>Index: layout/base/nsIPresShell.h
>+ // Helper func for accessibility
>+ void InvalidateAccessibleSubtree(nsIContent *aContent);
Shouldn't that be in #ifdef ACCESSIBILITY? The implementation certainly is.
In any case, it should be more clearly documented when this needs to be called.
>Index: layout/base/nsPresShell.cpp
>+void nsIPresShell::InvalidateAccessibleSubtree(nsIContent
>+ accService->InvalidateSubtreeFor(this, aContent,
>+ nsIAccessibleEvent::EVENT_REORDER);
Weird indent here.
You need to InvalidateAccessibleSubtree() in
nsIPresShell::ReconstructStyleDataInternal, per our IRC discussion -- it's
called in cases not caught by nsIDocumentObserver.
>+nsDocAccessible::AttributeChanged(nsIDocument *aDocument,
>+ if (aNameSpaceID == kNameSpaceID_StatesWAI_Unofficial) {
So we're not reacting to HTML attr changes at all anymore? I think we used
to...
>+void nsDocAccessible::ContentAppended(nsIDocument *aDocument,
InvalidateCacheSubtree(aContainer->GetChildAt(aNewIndexInContainer),
>+ nsIAccessibleEvent::EVENT_SHOW);
No, that's not good enough. ContentAppended can be called when multiple nodes
(at indices starting at aNewIndexInContainer) are all appended at once.
Not only that, but how do you know that's an EVENT_SHOW? The new node could be
display:none...
>+nsDocAccessible::ContentInserted(nsIDocument *aDocument,
>+ InvalidateCacheSubtree(aChild, nsIAccessibleEvent::EVENT_SHOW);
Again, how do you know it's SHOW?
Is InvalidateCacheSubtree async? If not, then for all these methods you might
invalidate the subtree before the change has actually happened, looks like...
That is, you're making assumptions about the ordering of your observer and the
presshell that are not guaranteed to be valid.
>+nsresult nsDocAccessible::FireDelayedToolkitEvent(PRUint32
>+ nsCOMPtr<nsIAccessibleEvent> event =
>+ new nsAccessibleEventData(aEvent, aAccessible, this, aData);
>+ NS_ENSURE_TRUE(event, NS_ERROR_OUT_OF_MEMORY);
You want to null out the timer if you created one; otherwise the next call into
this will think the timer is started when it's actually not.
I have to wonder; why is this using a timer instead of just posting a PLEvent?
>Index: accessible/src/msaa/nsDocAccessibleWrap.cpp
>+ for (PRUint32 count = 0; supportedEvents[count] != 0xffff; count ++) {
Why not just use NS_ARRAY_LENGTH(supportedEvents) here?
I can't reasonably review most of the accessibility changes other than the ones
to use document observer; someone familiar-ish with that code should have a
look.
Attachment #183926 -
Flags: superreview?(bzbarsky)
Attachment #183926 -
Flags: superreview-
Attachment #183926 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 21•20 years ago
|
||
Bz once these changes are okay for you, I will try to find someone more
familiar with the accessibility module for r=
>@@ -11596,45 +11635,23 @@ nsCSSFrameConstructor::RecreateFramesFor
> Is InvalidateCacheSubtree async? If not, then for all these methods you
might
> invalidate the subtree before the change has actually happened, looks like...
> That is, you're making assumptions about the ordering of your observer and
the
> presshell that are not guaranteed to be valid.
We use FireDelayedToolkitEvent to fire the event_show and event_reorder so that
the accessible tree is not rebuilt until the changes have occured.
>+nsresult nsDocAccessible::FireDelayedToolkitEvent(PRUint32
>>+ nsCOMPtr<nsIAccessibleEvent> event =
>>+ new nsAccessibleEventData(aEvent, aAccessible, this, aData);
>>+ NS_ENSURE_TRUE(event, NS_ERROR_OUT_OF_MEMORY);
> You want to null out the timer if you created one; otherwise the next call
into
> this will think the timer is started when it's actually not.
We don't need do that. If FlushEventsCallback has already happened, then
mEventsToFire will be empty, and isTimerStarted is set to false. This will
allow
the timer to be restarted for each new set of events without recreating mTimer
each time.
> I have to wonder; why is this using a timer instead of just posting a
PLEvent?
I have not yet used this technique. We use a zero length timer in several
places in
the accessibility module, including the previous logic for delayed events we're
just
moving around in this patch. I suggest an offshoot bug to look at using the
PLEvent
technique in the accessibility module in general, if it is the better way.
>>+ if (aNameSpaceID == kNameSpaceID_StatesWAI_Unofficial) {
> So we're not reacting to HTML attr changes at all anymore? I think we used
> to...
We never needed to. It's redundant with older code that watched for those
changes.
For example, we listen to CheckboxStateChange from nsHTMLInpoutElement
> As written, the changes in this patch will miss some show/hide events (eg if
> visibility of both a parent and a child changes we can dispatch a hide on the
> parent while the child remains visible).
I would like to live with that for now, and file a follow-up bug.
>+void nsDocAccessible::ContentAppended(nsIDocument *aDocument,
>InvalidateCacheSubtree(aContainer->GetChildAt(aNewIndexInContainer),
>+ nsIAccessibleEvent::EVENT_SHOW);
>> Not only that, but how do you know that's an EVENT_SHOW? The new node could
be
display:none...
InvalidateCacheSubtree will not fire the EVENT_SHOW for the new node unless an
accessible can be created for the passed in node, which it can't do unless the
node is visible. The right thing happens there so no need for an extra
visibility
check here.
I will add that as a comment for ContentAppended() and ContentInserted()
The converse is also true. If content is removed no hide event is fired unless
a
accessible existed for the content, which means it had to be visible.
Assignee | ||
Updated•20 years ago
|
Attachment #183926 -
Attachment is obsolete: true
Attachment #185112 -
Flags: superreview?
![]() |
||
Comment 22•20 years ago
|
||
> We use FireDelayedToolkitEvent
That's not what I meant. For example, say you get a ContentAppended. You go
through nsDocAccessible::InvalidateCacheSubtree and call
GetAccessibleInWeakShell() in the |aChangeEventType ==
nsIAccessibleEvent::EVENT_SHOW| branch. GetAccessibleInWeakShell calls
|aPresShell->GetPrimaryFrameFor(content, &frame);| and if there is no frame
returns null. But the content is just being added to the document, and
observers are just being notified. Both the presshell and you are observers and
the presshell will construct the frame when it gets the notification. So you're
assuming that you come after the presshell in the observer list here; otherwise
the frame will always be null. This assumption may be true sometimes, but in
general that's not guaranteed.
The rest seems reasonable... haven't had a chance to read the diff completely,
yet, though.
![]() |
||
Updated•20 years ago
|
Attachment #185112 -
Flags: superreview? → superreview?(bzbarsky)
Assignee | ||
Comment 23•20 years ago
|
||
(In reply to comment #22)
> > We use FireDelayedToolkitEvent
>
> That's not what I meant. For example, say you get a ContentAppended. You go
> through nsDocAccessible::InvalidateCacheSubtree and call
Okay, I will work on that tomorrow and any other problems you have time to
notice before then. Thank you for helping on this.
Assignee | ||
Comment 24•20 years ago
|
||
Attachment #185112 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #185112 -
Flags: superreview?(bzbarsky)
Assignee | ||
Updated•20 years ago
|
Attachment #185184 -
Flags: superreview?(bzbarsky)
Attachment #185184 -
Flags: review?(Louie.Zhao)
![]() |
||
Comment 25•20 years ago
|
||
Comment on attachment 185184 [details] [diff] [review]
Make FireDelayedToolkitEvent take dom node for event instead of accessible. Thus, we avoid problems with creating an accessible too early, before the node's frame had been updated.
>Index: layout/base/nsIPresShell.h
>+ void InvalidateAccessibleSubtree(nsIContent *aContent);
nsIPresShell is a "public" layout interface (as public as they get, except for
frozen ones). Does this method really belong here? Would it ever be called
from outside layout?
Based on the way it's used, it looks to me like this should be a private method
on nsPresShell.
>Index: accessible/src/base/nsAccessibleEventData.cpp
>+NS_IMETHODIMP nsAccessibleEventData::GetAccessible(nsIAccessible **aAccessible)
>+ NS_ASSERTION(mDOMNode, "Not initialized with an accessible or DOM Node!");
>+ NS_ENSURE_TRUE(mDOMNode, NS_ERROR_FAILURE);
Why bother with the null-check if the assert is there? If this can happen, the
assert should be a warning (which NS_ENSURE_TRUE provides); if it can't the
assert should just be an assert and the null-check should go (imo).
>+ NS_ASSERTION(accService, "No accessibility service");
>+ NS_ENSURE_TRUE(accService, NS_ERROR_FAILURE);
Same.
>+NS_IMETHODIMP nsAccessibleEventData::GetDOMNode(nsIDOMNode **aDOMNode)
>+ NS_ASSERTION(accessNode, "Not initialized with an accessible or DOM Node!");
>+ NS_ENSURE_TRUE(accessNode, NS_ERROR_FAILURE);
Same.
>Index: accessible/src/base/nsAccessibleEventData.h
>+ nsAccessibleEventData(PRUint32 aEventType, nsIDOMNode *aDOMNode,
>+ nsIAccessibleDocument *aDocAccessible,
>+ void *aEventData);
Is this mis-indented, or just tab-indented (and if the latter, is the whole
file?).
>Index: accessible/src/base/nsDocAccessible.cpp
>@@ -592,14 +570,8 @@ nsresult nsDocAccessible::RemoveEventLis
>+ // add document observer
>+ mDocument->RemoveObserver(this);
Fix the comment?
>+nsDocAccessible::AttributeChanged(nsIDocument *aDocument, nsIContent*
>+ docShell->GetBusyFlags(&busyFlags);
>+ if (busyFlags) {
>+ return; // Still loading, ignore setting of initial attributes
>+ }
I'm not sure I follow this part. If script changes attributes while a document
is loading, do you really want to ignore it? Do you rebuild the entire
accessuble tree onload or something?
If you're just trying to ignore attribute sets that come from the parser, then
you're in luck -- those don't call AttributeChanged in the first place. ;)
>+void nsDocAccessible::ContentAppended(nsIDocument *aDocument,
>+ InvalidateCacheSubtree(aContainer->GetChildAt(index),
>+ nsIAccessibleEvent::EVENT_SHOW);
Fix indent?
>+NS_IMETHODIMP nsDocAccessible::InvalidateCacheSubtree(nsIContent *aChild,
>+ // Don't fire any other events if doc is still loading
Same question as for AttributeChanged.
>+ // Don't go any high in parent chain than this
"higher"
>Index: accessible/src/base/nsRootAccessible.cpp
> privAcc->FireToolkitEvent(nsIAccessibleEvent::EVENT_VALUE_CHANGE,
>- accessible, nsnull);
>+ accessible, nsnull);
Fix indent, please.
sr=bzbarsky with these issues addressed.
Attachment #185184 -
Flags: superreview?(bzbarsky) → superreview+
Assignee | ||
Comment 26•20 years ago
|
||
Attachment #183116 -
Attachment is obsolete: true
Assignee | ||
Comment 27•20 years ago
|
||
Some more good testcases:
http://www.mozilla.org/access/dhtml/alert
http://www.mozilla.org/access/dhtml/spreadsheet (the menupopups should fire
EVENT_SHOW)
![]() |
||
Comment 28•20 years ago
|
||
Comment on attachment 185184 [details] [diff] [review]
Make FireDelayedToolkitEvent take dom node for event instead of accessible. Thus, we avoid problems with creating an accessible too early, before the node's frame had been updated.
The a11y part of this patch is OK. I have tested it against Linux. Although a
few minor issues exists, it's all related to ATK. I will take care of it in
another bug.
Attachment #185184 -
Flags: review?(Louie.Zhao) → review+
Assignee | ||
Comment 29•20 years ago
|
||
Comment on attachment 185184 [details] [diff] [review]
Make FireDelayedToolkitEvent take dom node for event instead of accessible. Thus, we avoid problems with creating an accessible too early, before the node's frame had been updated.
This patch is necessary for DHTML accessibility: see
http://www.mozilla.org/access/dhtml for more info.
Layout changes are wrapped in if (presShell->IsAccessibilityActive()) -- other
than the reomval of the nsChangeHint_AccessibleChange which is basically
reverting back to what we had before.
I will address bz's comments in final checkin, and post the checkin patch then.
Attachment #185184 -
Flags: approval1.8b3?
![]() |
||
Comment 30•20 years ago
|
||
Actually, I'd appreciate seeing the updated patch (or specifically the
resolution of the "document is loading" issue).
Assignee | ||
Comment 31•20 years ago
|
||
Comment on attachment 185184 [details] [diff] [review]
Make FireDelayedToolkitEvent take dom node for event instead of accessible. Thus, we avoid problems with creating an accessible too early, before the node's frame had been updated.
Clearing flag until I can post another patch.
Attachment #185184 -
Flags: approval1.8b3?
Assignee | ||
Comment 32•20 years ago
|
||
Here's the reason for not firing accessibility events when the document is
still loading:
For attribute changes, an onload handler may be used to set attributes like
checked on a span-based checkbox. It's not that a user action is causing the
checkbox to be checked when it was clear before, but it's the initial value
being set. This will often be the case in HTML because there is no way to
declare the namespaced attributes for DHTML accessibility. In that case an
onload script is used to initialize all of the checkboxes to the desired state.
We don't want to be firing events that may cause a screen reader to read
"checkbox checked" unless a user action is somehow causing it.
In the InvalidateCacheSubtree() we only return early if (busyFlags &&
mAccessNodeCache.Count() <= 1)
This is only the case if no AT has yet queried the current tree -- only the
document accessible (this) has been created which is why there is only 1 object
in the cache so far. So, we avoid extra thrashing and unnecessary accessible
creation during the mutations for document load, because it would serve no
purpose other than to hurt performance.
Assignee | ||
Updated•20 years ago
|
Attachment #185743 -
Attachment is obsolete: true
Assignee | ||
Comment 33•20 years ago
|
||
(In reply to comment #25)
> nsIPresShell is a "public" layout interface (as public as they get, except for
> frozen ones). Does this method really belong here? Would it ever be called
> from outside layout?
>
> Based on the way it's used, it looks to me like this should be a private
method on nsPresShell.
Problem is, it gets called from nsIPresShell::ReconstructStyleDataInternal() so
it either needs to stay the same, or be a public PresShell method in which case
I have to upcast to get to get my PresShell* version of |this|. Or am I missing
something?
I definitely don't understand why we need to implement methods on nsIPresShell
itself. Very un-COM like. Why are we breaking those rules so terribly? The
interface is supposed to define pure virtual methods using = 0, and then the
implementation class implements them.
Then we have this strange trio of ReconstructStyleDataExternal(),
ReconstructStyleDataInternal() and ReconstructStyleData(), the last of which
depends on _IMPL_NS_LAYOUT whatever that is. It just calls one of the first 2
depending on whether that's defined. ReconstructStyleDataExternal() then
redirect everything to the internal method.
Bottom line is, how to I make InvalidateAccessibleSubtree() a private method on
PresShell if an nsIPresShell method needs to call it?
![]() |
||
Comment 34•20 years ago
|
||
> Problem is, it gets called from nsIPresShell::ReconstructStyleDataInternal()
Then make it a private nsIPresShell method, I guess....
> I definitely don't understand why we need to implement methods on nsIPresShell
Performance and footprint. This isn't a COM interface at this point, for
similar reasons.
> _IMPL_NS_LAYOUT whatever that is.
It's something defined in files that are linked into the layout module (and
which can therefore make faster non-virtual calls on presshell).
Assignee | ||
Comment 35•20 years ago
|
||
(In reply to comment #34)
> > Problem is, it gets called from nsIPresShell::ReconstructStyleDataInternal()
> Then make it a private nsIPresShell method, I guess....
But if I do that I can't access the method from PresShell. Don't I have to keep
it protected like it is?
Assignee | ||
Comment 36•20 years ago
|
||
Bz is this okay?
Assignee | ||
Comment 37•20 years ago
|
||
Attachment #185772 -
Attachment is obsolete: true
![]() |
||
Comment 38•20 years ago
|
||
Comment on attachment 185774 [details] [diff] [review]
Correct cleanup patch
sr=bzbarsky. I didn't realize the method was already protected; I just don't
want it public.
Attachment #185774 -
Flags: superreview+
Assignee | ||
Updated•20 years ago
|
Attachment #185774 -
Flags: approval1.8b3?
Updated•20 years ago
|
Attachment #185774 -
Flags: approval1.8b3? → approval1.8b3+
Assignee | ||
Comment 39•20 years ago
|
||
Checking in layout/base/nsCSSFrameConstructor.cpp;
/cvsroot/mozilla/layout/base/nsCSSFrameConstructor.cpp,v <--
nsCSSFrameConstructor.cpp
new revision: 1.1102; previous revision: 1.1101
done
Checking in layout/base/nsCSSFrameConstructor.h;
/cvsroot/mozilla/layout/base/nsCSSFrameConstructor.h,v <-- nsCSSFrameConstructor.h
new revision: 1.185; previous revision: 1.184
done
Checking in layout/base/nsChangeHint.h;
/cvsroot/mozilla/layout/base/nsChangeHint.h,v <-- nsChangeHint.h
new revision: 3.10; previous revision: 3.9
done
Checking in layout/base/nsIPresShell.h;
/cvsroot/mozilla/layout/base/nsIPresShell.h,v <-- nsIPresShell.h
new revision: 3.156; previous revision: 3.155
done
Checking in layout/base/nsPresShell.cpp;
/cvsroot/mozilla/layout/base/nsPresShell.cpp,v <-- nsPresShell.cpp
new revision: 3.840; previous revision: 3.839
done
Checking in layout/style/nsStyleContext.cpp;
/cvsroot/mozilla/layout/style/nsStyleContext.cpp,v <-- nsStyleContext.cpp
new revision: 3.259; previous revision: 3.258
done
Checking in layout/style/nsStyleStruct.cpp;
/cvsroot/mozilla/layout/style/nsStyleStruct.cpp,v <-- nsStyleStruct.cpp
new revision: 3.128; previous revision: 3.127
done
Checking in accessible/public/nsIAccessibilityService.idl;
/cvsroot/mozilla/accessible/public/nsIAccessibilityService.idl,v <--
nsIAccessibilityService.idl
new revision: 1.47; previous revision: 1.46
done
Checking in accessible/public/nsIAccessibleEvent.idl;
/cvsroot/mozilla/accessible/public/nsIAccessibleEvent.idl,v <--
nsIAccessibleEvent.idl
new revision: 1.5; previous revision: 1.4
done
Checking in accessible/public/nsPIAccessibleDocument.idl;
/cvsroot/mozilla/accessible/public/nsPIAccessibleDocument.idl,v <--
nsPIAccessibleDocument.idl
new revision: 1.5; previous revision: 1.4
done
Checking in accessible/src/base/nsAccessibilityAtomList.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityAtomList.h,v <--
nsAccessibilityAtomList.h
new revision: 1.22; previous revision: 1.21
done
Checking in accessible/src/base/nsAccessibilityService.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityService.cpp,v <--
nsAccessibilityService.cpp
new revision: 1.143; previous revision: 1.142
done
Checking in accessible/src/base/nsAccessibleEventData.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibleEventData.cpp,v <--
nsAccessibleEventData.cpp
new revision: 1.6; previous revision: 1.5
done
Checking in accessible/src/base/nsAccessibleEventData.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibleEventData.h,v <--
nsAccessibleEventData.h
new revision: 1.9; previous revision: 1.8
done
Checking in accessible/src/base/nsDocAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsDocAccessible.cpp,v <-- nsDocAccessible.cpp
new revision: 1.62; previous revision: 1.61
done
Checking in accessible/src/base/nsDocAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsDocAccessible.h,v <-- nsDocAccessible.h
new revision: 1.27; previous revision: 1.26
done
Checking in accessible/src/base/nsRootAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsRootAccessible.cpp,v <--
nsRootAccessible.cpp
new revision: 1.120; previous revision: 1.119
done
Checking in accessible/src/base/nsRootAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsRootAccessible.h,v <-- nsRootAccessible.h
new revision: 1.49; previous revision: 1.48
done
Checking in accessible/src/html/nsHTMLLinkAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLLinkAccessible.cpp,v <--
nsHTMLLinkAccessible.cpp
new revision: 1.18; previous revision: 1.17
done
Checking in accessible/src/html/nsHTMLLinkAccessible.h;
/cvsroot/mozilla/accessible/src/html/nsHTMLLinkAccessible.h,v <--
nsHTMLLinkAccessible.h
new revision: 1.18; previous revision: 1.17
done
Checking in accessible/src/html/nsHTMLTextAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLTextAccessible.cpp,v <--
nsHTMLTextAccessible.cpp
new revision: 1.42; previous revision: 1.41
done
Checking in accessible/src/html/nsHTMLTextAccessible.h;
/cvsroot/mozilla/accessible/src/html/nsHTMLTextAccessible.h,v <--
nsHTMLTextAccessible.h
new revision: 1.31; previous revision: 1.30
done
Checking in accessible/src/msaa/nsDocAccessibleWrap.cpp;
/cvsroot/mozilla/accessible/src/msaa/nsDocAccessibleWrap.cpp,v <--
nsDocAccessibleWrap.cpp
new revision: 1.29; previous revision: 1.28
done
Checking in accessible/src/xul/nsXULMenuAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULMenuAccessible.cpp,v <--
nsXULMenuAccessible.cpp
new revision: 1.35; previous revision: 1.34
done
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•