Closed Bug 720987 Opened 13 years ago Closed 12 years ago

Focus rings keep growing when repeatedly tabbing through elements

Categories

(Core :: Web Painting, defect)

Product:
Core
Component:
Web Painting
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla12
Tracking Flags:
Tracking Status
firefox12 - ---
firefox13 - ---
firefox14 - ---

People

(Reporter: dao, Assigned: MatsPalmgren_bugz)

References

(Blocks 1 open bug)

Details

(Keywords: regression, relnote)

Attachments

(2 files)

testcase
(deleted), text/html
Details
fix + test
(deleted), patch
roc
: review+
Details | Diff | Splinter Review
Attached file testcase (deleted) — 
      No description provided.
Probably caused by bug 524925 or bug 665597?
Assignee: nobody → matspal
Blocks: 524925
Blocks: 719177
Attached patch fix + test (deleted) — — Splinter Review 
We shouldn't use the currently stored overflow areas for input to
FinishAndStoreOverflow since it expects the current frame rect +
child overflow to start from.  UpdateOverflow() does it right.
Attachment #591691 - Flags: review?(roc)
Comment on attachment 591691 [details] [diff] [review]
fix + test

Review of attachment 591691 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/bugs/720987.html
@@ +19,5 @@
> +  document.documentElement.removeAttribute("class");
> +}
> +</script>
> +</head>
> +<body style="padding:100px;" onload="setTimeout(doTest,0)">

Actually, this should use MozReftestInvalidate to make sure the document is rendered before doTest runs.

Hmm, do you have other tests doing this?
# grep -r 'onload="setTimeout' layout/reftests/ | wc -l
57

I guess some of those should probably use MozReftestInvalidate so I filed bug 721940
https://hg.mozilla.org/integration/mozilla-inbound/rev/731208933852
Flags: in-testsuite+
Whiteboard: [inbound]
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/731208933852
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Depends on: 722325
This caused bug #722325, which made the native fennec browser unusable. To fix bug #722325, a partial revert was checked in (it reverts this change for transformed frames).

It seems that UpdateOverflow is incorrect somewhere with respect to CSS transforms?

Native fennec uses CSS translate + nsIFrameLoaderOwnder:clipSubdocument to buffer off-screen areas of an element, and this patch caused areas that should have been invalidated not to be invalidated.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Tracking for FF12 - we'd definitely consider taking a for this during Aurora 12.
tracking-firefox12: ? → +
Mats - can you take a look at what caused the partialbackout?

Also adding qawanted to see if the partial revert caused this behavior to occur again.
Keywords: qawanted
(In reply to Alex Keybl [:akeybl] from comment #11)
> Mats - can you take a look at what caused the partialbackout?
> 
> Also adding qawanted to see if the partial revert caused this behavior to
> occur again.

How do we test a partial backout? Is this specific to Fennec?
Running the attached testcase in Firefox 11.0 and 12.0b2, the focus ring remains the same size. All I'm doing is loading the testcase and letting it run. Am I doing something wrong?
Bump. Please advise RE comment 13.
Retested this as per Jesse's instructions by testing in Nightly from 2012-01-25 then again in 12b3.

Nightly 12.0a1 2012-01-25 reproduces the bug; Beta 12.0b3 does not.
Thanks Anthony - we'll track for FF14 in that case.
tracking-firefox12: + → -
tracking-firefox14: --- → +
We've seen new dupes of this issue - tracking for FF13.
tracking-firefox13: --- → +
At least in Firefox 13.0b1, I cannot reproduce the attached testcase. Is there anything QA can do?
Keywords: qawanted
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #19)
> At least in Firefox 13.0b1, I cannot reproduce the attached testcase. Is
> there anything QA can do?

If you're no longer able reproduce, no need to track for release. Would be good to find out if we can also untrack for FF14 at this point.
tracking-firefox13: + → -
(In reply to Alex Keybl [:akeybl] from comment #20)
> If you're no longer able reproduce, no need to track for release. Would be
> good to find out if we can also untrack for FF14 at this point.

Strangely, in Firefox 14.0a2 2012-05-14, I don't get any focus rings (Win7 x64).
[Triage Comment]
I'm not sure if comment 21 is an affirmation that this can't be reproduced in 14, so flagging qawanted again just to confirm if we can untrack this for 14.
Keywords: qawanted
I repeated my testing. Firefox 14.0a2 2012-06-03 does not display any focus rings with the attached testcase. There's nothing more QA can do here without a different testcase or more specific instructions. I'm not exactly sure what you are looking for, Lukas.
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #23)
> I repeated my testing. Firefox 14.0a2 2012-06-03 does not display any focus
> rings with the attached testcase.

What I was looking for was a statement of whether not displaying focus rings was the desired result here. Is that equal to "does not reproduce"?  The bug summary claims focus rings keep growing and so it's not clear to me that no focus rings is the desired state and not another bug of 'no focus rings'. Does that make sense?
Here is what I am seeing, just to reiterate:
* Firefox 12.0b3: focus rings display but are the same size as the test goes on
* Firefox 12.0a1: focus rings display and they grow in size as the test goes on
* Firefox 14.0a2: focus rings do not display

The only conclusions I can make from this is that the fix is verified Firefox 12 and unverifiable in Firefox 14. In other words, does not reproduce != verified fixed. Sorry if that did not come across clearly in my previous comments. 

I'm not sure there is anything more QA can do here. Either the testcase is broken for Aurora or there is a bug in Aurora which needs to be resolved before we can verify this bug.

Dropping QAWANTED -- please re-add if there's something specific QA can do to help here.
Keywords: qawanted
Blocks: 764554
Strangely, the focus rings are now appearing across all builds. I can no longer reproduce bug 764554 (see https://bugzilla.mozilla.org/show_bug.cgi?id=764554#c4). I think we can resolve that bug WORKSFORME. Additionally, I think that means we can stop tracking this bug; unless there is something more you want tested.
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #26)
> Strangely, the focus rings are now appearing across all builds. I can no
> longer reproduce bug 764554 (see
> https://bugzilla.mozilla.org/show_bug.cgi?id=764554#c4). I think we can
> resolve that bug WORKSFORME. Additionally, I think that means we can stop
> tracking this bug; unless there is something more you want tested.

Now that you're able to see the focus rings on 14, have you been able to reproduce the issue where focus rings grow when tabbing between elements?
Keywords: qawanted
No, as reported in comment 25, I've not been able to reproduce the focus ring growth on anything bug 12.0 Nightly. I've not been able to reproduce this issue on any Firefox 12 Aurora, Firefox 12 Beta, Firefox 13, 14, 15, or 16 builds. 

I don't see that there is anything left for QA to do here. If there are real-world users reporting this issue on anything other than 12.0 Nightly then I suggest the next action should be to reach out to them.

From my perspective, this is FIXED.
Keywords: qawanted
Thanks Anthony
tracking-firefox14: + → -
The partial backout means this bug is not fixed for elements with transforms, but I believe bug 725664 covers that, so this bug should probably be marked fixed again.
or we could just remove this unnecessary vendor element.

1. it causes unexpected behavior (additional padding)
2. it’s mentioned in no standard
3. it’s unnecessare in order to achieve the desired effect, a outline with negative offset will do fine

this would be bug 140562
Status: REOPENED → RESOLVED
Closed: 13 years ago → 12 years ago
Resolution: --- → FIXED
This patch introduced the problem I describe in the paragraph beginning "We've also consistently had very odd handling" in bug 984226 comment 0.  I think it should be refixed as I describe in bug 984226 comment 3.
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: