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

test and implement correct behavior of margin/border/padding on ruby elements

RESOLVED FIXED in mozilla38

Status

()

Core
Layout: Block and Inline
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dbaron, Assigned: xidorn)

Tracking

Trunk
mozilla38
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(9 attachments, 6 obsolete attachments)

1.10 KB, text/html
Details
63.72 KB, image/png
Details
4.51 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
5.84 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
7.23 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
2.60 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
6.31 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
4.28 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
8.83 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
(Reporter)

Description

3 years ago
We should add tests for the correct behavior of margin, border, and padding on ruby elements.  It's not completely clear what they should do, particularly in the vertical direction (where they could be more like inlines or more like inline-blocks, although I'd prefer more like inlines).  This may depend on the spec specifying correct behavior.

The current code may well be in pretty good shape, though.
Created attachment 8535289 [details]
testcase 1 (border on various ruby elements)

Here's a testcase with a border on various <ruby> elements.

Right now, a sufficiently-large border on any ruby element will overlap either that element's own contents, or the contents of an adjacent element. Some of this probably needs to be addressed before enabling ruby -- particularly in cases where the border overlaps the same element's own content.

I tried a similar local example with <span> elements (and no ruby), and there, it looks like normal inline-level elements' borders can cause similar overlap in the block direction (so, the border may overlap content in the previous/next line), but not in the inline direction (so, a <span>'s border won't overlap content to the right of it on the same line).
Created attachment 8535291 [details]
screenshot of testcase 1 (w/ cases of bad overlap highlighted)

Here's a screenshot of testcase 1, with the particularly-needing-to-be-fixed cases of overlap highlighted w/ arrows.  (where "particularly-needing-to-be-fixed" is defined as "inline-direction", based on comparison to <span> behavior as discussed in previous comment.)

Note that in the "border on rtc" example, the "xx" and "yy" characters themselves are overlapping a bit. (i.e. one <rt> is overlapping the next <rt>, squeezed by their <rtc>.)
(Assignee)

Updated

3 years ago
Depends on: 1055658
(Assignee)

Updated

3 years ago
No longer depends on: 1055658
(Assignee)

Comment 3

3 years ago
My plan of implementing this bug is:

1. Allows margin/border/padding on rb/rt boxes.
They are simply subclass of nsInlineFrame, hence it is almost done.

2. Suppress border/padding on rbc/rtc boxes.
It is not trivial to implement border/padding on these boxes.
These boxes behave like inline frames, especially given that they can be broken inside. However, we still have bug 122795 open, which means our border/padding support for inline frames is not that complete yet. I don't think it worth adding another immature impl here.
In addition, we can't even keep the current immature impl, because whatever padding/border added, in my opinion, it is still necessary to keep content of the boxes aligned among levels. This will require a bit more code than just an simple immature impl.

3. Allows margin on rbc/rtc boxes.
The inline direction margin has been handled by nsLineLayout, and the block direction could be handled in nsRubyFrame manually. I'm not sure should I do this. It seems that block direction margin has no effect on inline elements?

4. Allow margin on ruby boxes.
It has been handled by nsLineLayout. I don't think any additional thing needs to be done for it.

5. Leave the current immature impl of border/padding of ruby boxes.
Same reason with rbc/rtc boxes. But the current immature impl for ruby boxes doesn't that broken when we don't break inside. And when there is a line break, it is just a little worse than the nsInlineFrame (padding-start is always applied here). So I think it's fine to leave it as it is now.

Do you agree with this plan? Or do you have any concerns about it?
Flags: needinfo?(dbaron)
(Reporter)

Comment 4

3 years ago
How does that match what the spec currently says?
Flags: needinfo?(dbaron)
(Assignee)

Comment 5

3 years ago
The spec discusses this problem in this section: http://dev.w3.org/csswg/css-ruby/#box-style

The spec does neither require nor forbid UAs to support any of border/padding/margin/background/outline on rbc and rtc. And other boxes should just behavior like a normal inline frame.

I think my plan completely matches the spec, except that <ruby> has a more flawed impl of border/padding than inline frame (which, I think, is easy to be fixed to as flawed as inline frame, but I wonder whether it is worth to fix now. I'd prefer we fix it when we get a mature impl for nsInlineFrame)

In addition, if the plan is to suppress border/padding on rbc/rtc, may I also suppress background/outline via overridding BuildDisplayList? Currently, the code for building display list for background and border seems to be bound together. I see no reason we should separate them just for rbc/rtc on which we are not required to implement these properties.
Flags: needinfo?(dbaron)
(Reporter)

Comment 6

3 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #3)
> My plan of implementing this bug is:
> 
> 1. Allows margin/border/padding on rb/rt boxes.
> They are simply subclass of nsInlineFrame, hence it is almost done.

OK.

> 2. Suppress border/padding on rbc/rtc boxes.
> It is not trivial to implement border/padding on these boxes.
> These boxes behave like inline frames, especially given that they can be
> broken inside. However, we still have bug 122795 open, which means our
> border/padding support for inline frames is not that complete yet. I don't
> think it worth adding another immature impl here.
> In addition, we can't even keep the current immature impl, because whatever
> padding/border added, in my opinion, it is still necessary to keep content
> of the boxes aligned among levels. This will require a bit more code than
> just an simple immature impl.

I don't think the existence of bug 122795 justifies this; it's quite obscure.  However, I'm OK with this anyway.

> 3. Allows margin on rbc/rtc boxes.
> The inline direction margin has been handled by nsLineLayout, and the block
> direction could be handled in nsRubyFrame manually. I'm not sure should I do
> this. It seems that block direction margin has no effect on inline elements?

That's correct; block-direction margin has no effect on inline elements.  So it sounds like there's nothing to do here.

> 4. Allow margin on ruby boxes.
> It has been handled by nsLineLayout. I don't think any additional thing
> needs to be done for it.

OK.

> 5. Leave the current immature impl of border/padding of ruby boxes.
> Same reason with rbc/rtc boxes. But the current immature impl for ruby boxes
> doesn't that broken when we don't break inside. And when there is a line
> break, it is just a little worse than the nsInlineFrame (padding-start is
> always applied here). So I think it's fine to leave it as it is now.

You should at least file a bug on fixing the border/padding always being applied after the break.  It doesn't need to block enabling ruby, but we should have a bug on file.



In addition to the above, you should have some tests testing the behavior.
Flags: needinfo?(dbaron)
(Reporter)

Comment 7

3 years ago
... and the tests could perhaps test backgrounds at the same time.
(Assignee)

Comment 8

3 years ago
So do you agree to suppress background and outline on rbc/rtc as well?
Flags: needinfo?(dbaron)
(Reporter)

Comment 9

3 years ago
I suppose I'd be OK with it; I'm not happy about it, though.  However, I think I might prefer supporting background and outline even if we're suppressing margin, border, and padding.
Flags: needinfo?(dbaron)
(Assignee)

Comment 10

3 years ago
So does it sound acceptable to have border painted without leaving space for it?
Flags: needinfo?(dbaron)
(Reporter)

Comment 11

3 years ago
No.
Flags: needinfo?(dbaron)
(Assignee)

Comment 12

3 years ago
The outline of ruby text boxes depends on the overflow area fix in bug 1055658.
Depends on: 1055658
(Assignee)

Comment 13

3 years ago
Created attachment 8547354 [details] [diff] [review]
patch 1 - Allow inline direction margin of ruby base/text boxes
Assignee: nobody → quanxunzhen
Attachment #8547354 - Flags: review?(dbaron)
(Assignee)

Comment 14

3 years ago
Created attachment 8547355 [details] [diff] [review]
patch 2 - Remove unnecessary param of SetBSizeFromFontMetrics
Attachment #8547355 - Flags: review?(dbaron)
(Assignee)

Comment 15

3 years ago
Created attachment 8547360 [details] [diff] [review]
patch 3 - Suppress border/padding space on rbc/rtc frames
Attachment #8547360 - Flags: review?(dbaron)
(Assignee)

Comment 16

3 years ago
Created attachment 8547361 [details] [diff] [review]
patch 4 - Add base class for rbc & rtc
Attachment #8547361 - Flags: review?(dbaron)
(Assignee)

Comment 17

3 years ago
Created attachment 8547362 [details] [diff] [review]
patch 5 - Stop building display items for borders on ruby level containers
Attachment #8547362 - Flags: review?(dbaron)
(Assignee)

Comment 18

3 years ago
Created attachment 8547363 [details] [diff] [review]
patch 7 - Reftests for box properties on ruby boxes
Attachment #8547363 - Flags: review?(dbaron)
(Assignee)

Updated

3 years ago
Attachment #8547363 - Attachment description: patch 7 - Fix border/padding of ruby frame → patch 7 - Reftests for box properties on ruby boxes
(Assignee)

Comment 19

3 years ago
Created attachment 8547369 [details] [diff] [review]
patch 6 - Fix border/padding of ruby frame
Attachment #8547369 - Flags: review?(dbaron)
(Assignee)

Comment 20

3 years ago
Created attachment 8547371 [details] [diff] [review]
patch 8 - Return zero as computed border/padding value for rbc/rtc
Attachment #8547371 - Flags: review?(dbaron)
(Reporter)

Updated

3 years ago
Attachment #8547354 - Flags: review?(dbaron) → review+
(Reporter)

Comment 21

3 years ago
Comment on attachment 8547355 [details] [diff] [review]
patch 2 - Remove unnecessary param of SetBSizeFromFontMetrics

Either you have a patch elsewhere in your queue removing the need for it, or you also need to change nsRubyBaseContainerFrame.

r=dbaron either way
Attachment #8547355 - Flags: review?(dbaron) → review+
(Reporter)

Comment 22

3 years ago
Comment on attachment 8547360 [details] [diff] [review]
patch 3 - Suppress border/padding space on rbc/rtc frames

I guess this is ok, but in addition to this (or maybe instead of it) you want to set border and padding to 0 in  nsCSSOffsetState::InitOffsets and then override GetUsedBorder and GetUsedPadding like nsTableFrame does.

The removal of the aReflowState parameter to nsLayoutUtils::SetBSizeFromFontMetrics belongs in the previous patch.

r=dbaron with that; I should probably review the additional changes as a separate patch
Attachment #8547360 - Flags: review?(dbaron) → review+
(Reporter)

Comment 23

3 years ago
Comment on attachment 8547361 [details] [diff] [review]
patch 4 - Add base class for rbc & rtc

Maybe comment next to the protected constructor that it is not expected to be instantiated on its own; only its derived classes are expected to be instantiated.

r=dbaron with that
Attachment #8547361 - Flags: review?(dbaron) → review+
(Reporter)

Comment 24

3 years ago
Comment on attachment 8547362 [details] [diff] [review]
patch 5 - Stop building display items for borders on ruby level containers

This seems like a future maintainability problem; it would be better not to create an extra copy of this code.

I wonder if the code that creates the border display item could detect that nothing should be done because of a GetUsedBorder override that returns 0 width?  (Might it already?)

It also breaks visibility:visible descendants of a visibility:hidden rtc/rbc, since the IsVisibleForPainting check is wrong.
Attachment #8547362 - Flags: review?(dbaron) → review-
(Assignee)

Comment 25

3 years ago
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #24)
> Comment on attachment 8547362 [details] [diff] [review]
> patch 5 - Stop building display items for borders on ruby level containers
> 
> This seems like a future maintainability problem; it would be better not to
> create an extra copy of this code.

So what should I do for this.

> I wonder if the code that creates the border display item could detect that
> nothing should be done because of a GetUsedBorder override that returns 0
> width?  (Might it already?)

It seems the answer is no, at least not already. I move the overriding code from patch 8, but the border is still painted. Any suggestion?
Flags: needinfo?(dbaron)
(Reporter)

Comment 26

3 years ago
Hmmm.  Is it really going to be simpler to suppress the border than to just implement border and padding correctly?
Flags: needinfo?(dbaron)
(Assignee)

Comment 27

3 years ago
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #26)
> Hmmm.  Is it really going to be simpler to suppress the border than to just
> implement border and padding correctly?

It is unclear what is the correct behavior for rbc and rtc, especially considering that they need to be aligned among levels and they also have aligned children.

There are several options, roughly either align the content boundary and move rtc or rbc accordingly, or align border boundary and extend the padding in some levels. The first opinion looks more correct, but requires more work to align, and could potentially make it harder to check line breaking. The second option at least requires one more loop to get the maximum border/padding.

The other reason I have mentioned is that it is unclear to me what will be done to make normal inline frames have proper border/padding behavior. Since the border/padding in ruby level containers would be more complicated than other inline frames, if we leave an immature impl here, we might need to do much more work when we want to fix it someday in the future.

Given the reasons, I still prefer to suppress instead of to implement them.

I didn't want to create an extra copy of that code, that's the reason I asked whether I can suppress background and outline as well.

There is another opinion to do the suppression, which is to compute border/padding to zero in nsRuleNode or nsStyleCotext::ApplyStyleFixups. That will cancel any potential effect of border/padding, and we can keep the related code together. Given that padding and border have their own style struct, I guess it would not be a big waste to fix them up in ApplyStyleFixups. Sounds good?
Flags: needinfo?(dbaron)
(Assignee)

Comment 28

3 years ago
s/opinion/option/ in the last paragraph...
(Assignee)

Comment 29

3 years ago
Created attachment 8548516 [details] [diff] [review]
patch to compute border/padding to zero for rbc/rtc

What about this? I guess it makes code more compact as we don't need to put the suppression everywhere to eliminate the effect.

I'm considering putting shared empty nsStyleBorder and nsStylePadding in nsPresContext, so that we don't need to create a new struct for every rbc and rtc.
Attachment #8548516 - Flags: feedback?(dbaron)
(Assignee)

Comment 30

3 years ago
The new patch can take place of the current patch 4 and patch 5.
(Reporter)

Comment 31

3 years ago
Comment on attachment 8547369 [details] [diff] [review]
patch 6 - Fix border/padding of ruby frame

This should only add the start-padding on if it's the first continuation or box-decoration-break is clone, and the end-padding on if it's reporting complete or box-decoration-break is clone.  (See nsInlineFrame::ReflowFrames, although you don't need to worry about the block-in-inline split cases, and I'm not sure why it tests !LastInFlow()->GetNextContinuation().)

You should also add reftests for both (a) what this is fixing and (b) some of the splitting cases.
Attachment #8547369 - Flags: review?(dbaron) → review-
(Reporter)

Comment 32

3 years ago
Comment on attachment 8547354 [details] [diff] [review]
patch 1 - Allow inline direction margin of ruby base/text boxes

You should also add a reftest for this.
(Assignee)

Comment 33

3 years ago
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #32)
> Comment on attachment 8547354 [details] [diff] [review]
> patch 1 - Allow inline direction margin of ruby base/text boxes
> 
> You should also add a reftest for this.

I have had reftests for all those things in patch 7.
(Reporter)

Comment 34

3 years ago
Comment on attachment 8547363 [details] [diff] [review]
patch 7 - Reftests for box properties on ruby boxes

r=dbaron, although I didn't look very closely.

(And, actually, maybe most of the reftests I requested in the previous two comments are covered here,except for the ones for the additional changes I asked for for patch 6.  Are they?)
Attachment #8547363 - Flags: review?(dbaron) → review+
(Assignee)

Comment 35

3 years ago
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #34)
> Comment on attachment 8547363 [details] [diff] [review]
> patch 7 - Reftests for box properties on ruby boxes
> 
> r=dbaron, although I didn't look very closely.
> 
> (And, actually, maybe most of the reftests I requested in the previous two
> comments are covered here,except for the ones for the additional changes I
> asked for for patch 6.  Are they?)

No, they aren't. I'll add.
(Reporter)

Comment 36

3 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #29)
> What about this? I guess it makes code more compact as we don't need to put
> the suppression everywhere to eliminate the effect.

I'll think about this.

> I'm considering putting shared empty nsStyleBorder and nsStylePadding in
> nsPresContext, so that we don't need to create a new struct for every rbc
> and rtc.

I don't think that's worth the code complexity; style struct ownership is complicated enough already.



One issue none of these suppression patches really handle is what to do if the theme (-moz-appearance) forces border or padding.  I guess we just don't worry about it, but it's not ideal.
(Reporter)

Comment 37

3 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #30)
> The new patch can take place of the current patch 4 and patch 5.

Also patch 8?
(Assignee)

Comment 38

3 years ago
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #37)
> (In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #30)
> > The new patch can take place of the current patch 4 and patch 5.
> 
> Also patch 8?

Yes, I forgot that.
(Reporter)

Comment 39

3 years ago
Comment on attachment 8548516 [details] [diff] [review]
patch to compute border/padding to zero for rbc/rtc

OK, let's do this instead of patches 4, 5, and 8.

It still doesn't handle the native theme issue, but I guess that's ok.  (The native theme issue might be a little better if we skipped patch 3, perhaps?)
Flags: needinfo?(dbaron)
Attachment #8548516 - Flags: review+
Attachment #8548516 - Flags: feedback?(dbaron)
Attachment #8548516 - Flags: feedback+
(Reporter)

Updated

3 years ago
Attachment #8547371 - Flags: review?(dbaron)
(Assignee)

Comment 40

3 years ago
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #39)
> Comment on attachment 8548516 [details] [diff] [review]
> patch to compute border/padding to zero for rbc/rtc
> 
> OK, let's do this instead of patches 4, 5, and 8.

I'm a bit concerned about the memory usage. I believe this impl always create a new struct even if we currently inherit an already empty one. Or maybe we always have an independent empty struct for reset style if it is not explicitly marked inherit?

> It still doesn't handle the native theme issue, but I guess that's ok.  (The
> native theme issue might be a little better if we skipped patch 3, perhaps?)

I need to look into what the theme would do for this.

I don't think thing would be better if we skip patch 3. I think it would just make the aligning odd, instead of simply have border without space for them.

Maybe we can also fixup -moz-appearance here?
Flags: needinfo?(dbaron)
(Reporter)

Comment 41

3 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #40)
> (In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from
> comment #39)
> > Comment on attachment 8548516 [details] [diff] [review]
> > patch to compute border/padding to zero for rbc/rtc
> > 
> > OK, let's do this instead of patches 4, 5, and 8.
> 
> I'm a bit concerned about the memory usage. I believe this impl always
> create a new struct even if we currently inherit an already empty one. Or
> maybe we always have an independent empty struct for reset style if it is
> not explicitly marked inherit?

You could restructure the patch to only call GetUniqueStyleData if the padding/border is nonzero.  Don't try any new ownership schemes, please.

> Maybe we can also fixup -moz-appearance here?

No; don't bother.
Flags: needinfo?(dbaron)
(Assignee)

Comment 42

3 years ago
Created attachment 8548577 [details] [diff] [review]
patch 4 - Compute border/padding to zero for rbc/rtc
Attachment #8547361 - Attachment is obsolete: true
Attachment #8547362 - Attachment is obsolete: true
Attachment #8547371 - Attachment is obsolete: true
Attachment #8548516 - Attachment is obsolete: true
Attachment #8548577 - Flags: review?(dbaron)
(Assignee)

Comment 43

3 years ago
Created attachment 8548639 [details] [diff] [review]
patch 5 - Make nsRubyFrame inherit nsInlineFrame

Simplifies some code stolen from nsInlineFrame. Also makes the frame act more like an inline frame in other methods we didn't copy.
(I wanted to copy GetLogicalSkipSides, but then I guess it would be better to make it inherit.)
Attachment #8548639 - Flags: review?(dbaron)
(Assignee)

Comment 44

3 years ago
Created attachment 8548640 [details] [diff] [review]
patch 6 - Fix border/padding of ruby frame
Attachment #8547369 - Attachment is obsolete: true
Attachment #8548640 - Flags: review?(dbaron)
(Assignee)

Comment 45

3 years ago
Created attachment 8548641 [details] [diff] [review]
patch 7 - Reftests for box properties on ruby boxes

Add one test for border/padding of ruby frames when there are line breaks.
Attachment #8547363 - Attachment is obsolete: true
Attachment #8548641 - Flags: review?(dbaron)
(Reporter)

Comment 46

3 years ago
Comment on attachment 8548577 [details] [diff] [review]
patch 4 - Compute border/padding to zero for rbc/rtc

>+    if (StyleBorder()->GetComputedBorder() != nsMargin()) {

>+        computedPadding != nsMargin()) {

I'd prefer that these be written as nsMargin(0, 0, 0, 0) rather than as nsMargin(), since it's clearer what the value being compared to is.

r=dbaron with that
Attachment #8548577 - Flags: review?(dbaron) → review+
(Reporter)

Comment 47

3 years ago
Comment on attachment 8548639 [details] [diff] [review]
patch 5 - Make nsRubyFrame inherit nsInlineFrame

>-  // The leadings required to put the annotations.
>+  // The leadings required to put the annotations. They are not
>+  // initialized until the first reflow.
>   nscoord mBStartLeading;
>   nscoord mBEndLeading;

This should say "The leading" rather than "The leadings", since leading is a mass noun.



This IsFrameOfType change is good, and reminds me that
nsRubyBaseContainerFrame should also return false for eSupportsCSSTransforms,
like nsInlineFrame and nsRubyTextContainerFrame do.  I filed bug 1121738 for this.  (I suspect it regressed at some previous inheritance change.)
Attachment #8548639 - Flags: review?(dbaron) → review+
(Reporter)

Comment 48

3 years ago
Comment on attachment 8548640 [details] [diff] [review]
patch 6 - Fix border/padding of ruby frame

r=dbaron, although I wonder if the nsCSSRendering code needs to handle some other frame types as well.
Attachment #8548640 - Flags: review?(dbaron) → review+
(Reporter)

Updated

3 years ago
Attachment #8548641 - Flags: review?(dbaron) → review+
(Assignee)

Comment 49

3 years ago
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #48)
> Comment on attachment 8548640 [details] [diff] [review]
> patch 6 - Fix border/padding of ruby frame
> 
> r=dbaron, although I wonder if the nsCSSRendering code needs to handle some
> other frame types as well.

I was using static_cast<nsInlineFrame*>(do_QueryFrame()), but I guess it might affect performance, and I don't see other subclasses of nsInlineFrame actually need this, so finally I changed it to this check.
(Reporter)

Comment 50

3 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #49)
> (In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from
> comment #48)
> > Comment on attachment 8548640 [details] [diff] [review]
> > patch 6 - Fix border/padding of ruby frame
> > 
> > r=dbaron, although I wonder if the nsCSSRendering code needs to handle some
> > other frame types as well.
> 
> I was using static_cast<nsInlineFrame*>(do_QueryFrame()), but I guess it
> might affect performance, and I don't see other subclasses of nsInlineFrame
> actually need this, so finally I changed it to this check.

Probably best to use either the do_QueryFrame test in both places, or maybe even use IsFrameOfType(eLineParticipant).  (I think what it's trying to test is whether the split boxes join logically in the inline or block directions, which makes me think eLineParticipant might be best.)
(Assignee)

Comment 51

3 years ago
Change that to using IsFrameOfType(eLineParticipant) might need a little more work than do_QueryFrame, since the function it calls assume the frame is a inline frame.

I'll make it use do_QueryFrame instead. I still suspect that it might cause some performance regression, though.
(Assignee)

Comment 52

3 years ago
try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f67dcd316abd
(Assignee)

Comment 53

3 years ago
inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/400423459c99
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0ce5974c7dd
https://hg.mozilla.org/integration/mozilla-inbound/rev/d220304dc8dd
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6f6cfd31f00
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e83e832b46b
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2f608193d38
https://hg.mozilla.org/integration/mozilla-inbound/rev/297f99f0ed18
https://hg.mozilla.org/mozilla-central/rev/400423459c99
https://hg.mozilla.org/mozilla-central/rev/a0ce5974c7dd
https://hg.mozilla.org/mozilla-central/rev/d220304dc8dd
https://hg.mozilla.org/mozilla-central/rev/f6f6cfd31f00
https://hg.mozilla.org/mozilla-central/rev/1e83e832b46b
https://hg.mozilla.org/mozilla-central/rev/f2f608193d38
https://hg.mozilla.org/mozilla-central/rev/297f99f0ed18
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(Assignee)

Updated

3 years ago
Blocks: 1135954
You need to log in before you can comment on or make changes to this bug.