Open
Bug 1322497
Opened 8 years ago
Updated 2 years ago
layout/generic/test/test_intrinsic_size_on_loading.html fails to run more than once in same browser session
Categories
(Core :: Layout, defect, P3)
Core
Layout
Tracking
()
NEW
People
(Reporter: malayaleecoder, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
2.00 KB,
patch
|
jmaher
:
review+
emk
:
review-
|
Details | Diff | Splinter Review |
./mach mochitest layout/generic/test/test_intrinsic_size_on_loading.html --repeat 1 fails.
Reporter | ||
Comment 1•8 years ago
|
||
Attachment #8817418 -
Flags: review?(jmaher)
Reporter | ||
Comment 2•8 years ago
|
||
This fix manages to avoid the error happening for multiple runs, but not a logical way. I tried removing the 8 loaded images just before finish, but on the successive run, initialOffsetTop takes the value of "8". Do let me know if there is a logical solution :)
Assignee: nobody → malayaleecoder
Status: NEW → ASSIGNED
Comment 3•8 years ago
|
||
Comment on attachment 8817418 [details] [diff] [review] Bug1322497_v1.diff Review of attachment 8817418 [details] [diff] [review]: ----------------------------------------------------------------- really close, lets do a few small cleanup items and get this back up for review! Thanks for working on this. ::: layout/generic/test/test_intrinsic_size_on_loading.html @@ +45,5 @@ > <script> > initialOffsetTop = marker.offsetTop; > + // Adjust the value of initialOffsetTop when running multiple times in same session > + if (initialOffsetTop == 8) > + initialOffsetTop = 0; A few small nits here- 1) the comment is too long, it should be split on multiple lines 2) in the future we will question why 8 is a magic number, that is just room for future failures- can we at least make it a const, or determine a way to reset when we finish or start the test? 3) I would prefer a initialize() or cleanup() method that we call, although for this case that would seem unecessary. good job on finding this, I had looked at this unsuccessfully the other day. When you update this, can you ask :emk for review?
Attachment #8817418 -
Flags: review?(jmaher) → review-
Reporter | ||
Comment 4•8 years ago
|
||
Have a look :)
Attachment #8817418 -
Attachment is obsolete: true
Attachment #8817451 -
Flags: review?(jmaher)
Attachment #8817451 -
Flags: review?(VYV03354)
Comment 5•8 years ago
|
||
Comment on attachment 8817451 [details] [diff] [review] Bug1322497_v2.diff Review of attachment 8817451 [details] [diff] [review]: ----------------------------------------------------------------- I am happier with this- lets see if :emk has other feedback.
Attachment #8817451 -
Flags: review?(jmaher) → review+
Comment 6•8 years ago
|
||
Comment on attachment 8817451 [details] [diff] [review] Bug1322497_v2.diff Review of attachment 8817451 [details] [diff] [review]: ----------------------------------------------------------------- If this test is repeated, it does not test what it is supposed to test. As written in the manual test case in bug 1205027, this test does not work if the image is already cached. To make the test repeatable, you will have to change file_SlowImage.sjs to 1. make images uncacheable and 2. add a "command" (like "?continue") to reinitialize the promise.
Attachment #8817451 -
Flags: review?(VYV03354) → review-
Updated•7 years ago
|
Priority: -- → P3
Comment 7•2 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: malayaleecoder → nobody
Status: ASSIGNED → NEW
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•