Intermittent bug917595-exif-rotated.jpg | image comparison (==), max difference: 224, number of differing pixels: 434985

RESOLVED DUPLICATE of bug 1251088

Status

()

Core
Layout: Images
RESOLVED DUPLICATE of bug 1251088
2 years ago
10 months ago

People

(Reporter: KWierso, Assigned: BenWa)

Tracking

({intermittent-failure})

39 Branch
x86_64
Mac OS X
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 affected, firefox46 affected)

Details

(Whiteboard: [rr-chaos])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

16:22:31 INFO - REFTEST INFO | [CONTENT] MozInvalidateEvent didn't invalidate
16:22:31 INFO - REFTEST TEST-LOAD | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug917595-1-ref.html | 11653 / 11944 (97%)
16:22:31 INFO - REFTEST INFO | [CONTENT] MozInvalidateEvent didn't invalidate
16:22:31 INFO - REFTEST fuzzy match
16:22:31 INFO - REFTEST TEST-PASS | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug917595-iframe-1.html | image comparison (==)
16:22:31 INFO - REFTEST INFO | Loading a blank page
16:22:31 INFO - REFTEST TEST-END | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug917595-iframe-1.html
16:22:31 INFO - REFTEST TEST-START | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug917595-exif-rotated.jpg
16:22:31 INFO - REFTEST TEST-LOAD | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug917595-exif-rotated.jpg | 11654 / 11944 (97%)
16:22:31 INFO - REFTEST TEST-LOAD | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug917595-pixel-rotated.jpg | 11654 / 11944 (97%)
16:22:32 INFO - REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug917595-exif-rotated.jpg | image comparison (==), max difference: 224, number of differing pixels: 434985

http://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-macosx64/1426886975/fx-team_snowleopard_test-reftest-bm106-tests1-macosx-build179.txt.gz&only_show_unexpected=1

16:22:32 INFO - REFTEST INFO | Loading a blank page
16:22:32 INFO - REFTEST TEST-END | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug917595-exif-rotated.jpg
16:22:32 INFO - REFTEST TEST-START | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug1106522-1.html
16:22:32 INFO - REFTEST TEST-LOAD | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug1106522-1.html | 11655 / 11944 (97%)
16:22:32 INFO - REFTEST TEST-LOAD | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug1106522-ref.html | 11655 / 11944 (97%)
16:22:32 INFO - REFTEST TEST-PASS | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug1106522-1.html | image comparison (==)
16:22:32 INFO - REFTEST INFO | Loading a blank page
16:22:32 INFO - REFTEST TEST-END | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug1106522-1.html
16:22:32 INFO - REFTEST TEST-START | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/dom/html/reftests/bug1106522-2.html
Comment hidden (Treeherder Robot)
Also happening on mulet tc: https://treeherder.allizom.org/#/jobs?repo=try&revision=794839d25b56&exclusion_profile=false
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 7

2 years ago
Inactive; closing (see bug 1180138).
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 28

2 years ago
7 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 2
* mozilla-central: 2
* b2g-inbound: 2
* fx-team: 1

Platform breakdown:
* osx-10-6: 4
* windows7-32: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2015-10-05&endday=2015-10-11&tree=all

Comment 29

2 years ago
8 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* b2g-inbound: 3
* fx-team: 1

Platform breakdown:
* osx-10-6: 6
* windows7-32: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2015-10-12&endday=2015-10-18&tree=all

Comment 30

2 years ago
10 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* fx-team: 3
* b2g-inbound: 3
* mozilla-inbound: 2
* mozilla-central: 2

Platform breakdown:
* windows7-32: 7
* osx-10-6: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2015-10-19&endday=2015-10-25&tree=all

Comment 31

2 years ago
12 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* mozilla-central: 4
* fx-team: 2
* b2g-inbound: 2

Platform breakdown:
* windows7-32: 8
* osx-10-6: 4

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2015-10-26&endday=2015-11-01&tree=all

Comment 32

2 years ago
11 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* fx-team: 3
* mozilla-central: 2
* b2g-inbound: 2

Platform breakdown:
* windows7-32: 6
* osx-10-6: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2015-11-02&endday=2015-11-08&tree=all

Comment 33

2 years ago
6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* fx-team: 3
* mozilla-inbound: 2
* b2g-inbound: 1

Platform breakdown:
* windows7-32: 3
* osx-10-6: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2015-11-16&endday=2015-11-22&tree=all

Comment 34

2 years ago
8 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 7
* mozilla-central: 1

Platform breakdown:
* windows7-32: 5
* osx-10-6: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2015-11-30&endday=2015-12-06&tree=all

Comment 35

2 years ago
6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 2
* b2g-inbound: 2
* mozilla-central: 1
* fx-team: 1

Platform breakdown:
* windows7-32: 5
* osx-10-6: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2015-12-07&endday=2015-12-13&tree=all
status-firefox45: --- → affected
status-firefox46: --- → affected
6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* fx-team: 3
* try: 1
* mozilla-inbound: 1
* b2g-inbound: 1

Platform breakdown:
* windows7-32: 3
* osx-10-6: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2016-01-18&endday=2016-01-24&tree=all
9 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 6
* fx-team: 2
* mozilla-central: 1

Platform breakdown:
* osx-10-6: 6
* windows7-32: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2016-02-01&endday=2016-02-07&tree=all
5 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 3
* mozilla-central: 1
* mozilla-beta: 1

Platform breakdown:
* windows7-32: 3
* osx-10-6: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2016-02-08&endday=2016-02-14&tree=all
(Assignee)

Comment 39

a year ago
I was able to reproduce this using rr chaos using only 17 runs.
Whiteboard: [rr-chaos]
(Assignee)

Comment 40

a year ago
I wonder if it's a similar issue to bug 1226748.
(Assignee)

Comment 41

a year ago
Created attachment 8722234 [details]
MozReview Request: Bug 1145903 - Use CSS to shrinkToFit instead of a racy set of width/height on the image frame. r=seth

Review commit: https://reviewboard.mozilla.org/r/35927/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35927/
Attachment #8722234 - Flags: review?(seth)
(Assignee)

Comment 42

a year ago
Thanks to RR for this bug!

Basically what's happening here is we set the size of the image frame when we get the 'load' event on the image document. However we haven't loaded the CSS style sheet yet which we need to get the 'image-orientation' style. During the race condition we don't have 'image-orientation' so we ignore the exif rotation from the image.

By using CSS to shrinkToFit we don't need to size the image frame and thus CSS updates the size properly when the CSS style is loaded.
Assignee: nobody → bgirard
(In reply to Benoit Girard (:BenWa) from comment #42)
> Basically what's happening here is we set the size of the image frame when
> we get the 'load' event on the image document. However we haven't loaded the
> CSS style sheet yet

If the document has had the load event, then shouldn't any style sheets that it is using also be loaded, parsed, and applied?
(Assignee)

Comment 44

a year ago
(In reply to Timothy Nikkel (:tnikkel) from comment #43)
> (In reply to Benoit Girard (:BenWa) from comment #42)
> > Basically what's happening here is we set the size of the image frame when
> > we get the 'load' event on the image document. However we haven't loaded the
> > CSS style sheet yet
> 
> If the document has had the load event, then shouldn't any style sheets that
> it is using also be loaded, parsed, and applied?

Good question. I haven't had time to look into that part. There's really two problems here. There's the data being wrong at the load events, and there's the data being static after it changes. My patch fixes the second part which is the most important. But it would be nice to fix the 'load' event as well. It needs more investigation.
(Assignee)

Comment 45

a year ago
Comment on attachment 8722234 [details]
MozReview Request: Bug 1145903 - Use CSS to shrinkToFit instead of a racy set of width/height on the image frame. r=seth

:tn do you mind reviewing this?
Attachment #8722234 - Flags: review?(seth) → review?(tnikkel)
Comment on attachment 8722234 [details]
MozReview Request: Bug 1145903 - Use CSS to shrinkToFit instead of a racy set of width/height on the image frame. r=seth

https://reviewboard.mozilla.org/r/35927/#review32963
Attachment #8722234 - Flags: review?(tnikkel) → review+
(In reply to Timothy Nikkel (:tnikkel) from comment #43)
> (In reply to Benoit Girard (:BenWa) from comment #42)
> > Basically what's happening here is we set the size of the image frame when
> > we get the 'load' event on the image document. However we haven't loaded the
> > CSS style sheet yet
> 
> If the document has had the load event, then shouldn't any style sheets that
> it is using also be loaded, parsed, and applied?

This sounds like a serious bug if true. We should investigate this while you have a trace.
(In reply to Timothy Nikkel (:tnikkel) from comment #43)
> (In reply to Benoit Girard (:BenWa) from comment #42)
> > Basically what's happening here is we set the size of the image frame when
> > we get the 'load' event on the image document. However we haven't loaded the
> > CSS style sheet yet
> 
> If the document has had the load event, then shouldn't any style sheets that
> it is using also be loaded, parsed, and applied?

If it was the load event for the image, and not the image _document_, that might make more sense.
(Assignee)

Comment 49

a year ago
Its for the image document:
http://mxr.mozilla.org/mozilla-central/source/dom/html/ImageDocument.cpp#596
The listener for that appears to be added here:

http://mxr.mozilla.org/mozilla-central/source/dom/html/ImageDocument.cpp#261

so it's listening for the load event of the img element. So then it makes sense.
(Assignee)

Comment 51

a year ago
So you're saying this matches with what I saw in the replay? Do we still need to file a new bug to improve the semantics of the 'load' event for ImageDocument?

I wonder how much work it would be to remove the 'little' ammount that happens on that 'load' event. I'm not really the best person to do it honestly. But right now the image document is trying to keep the image size up to date but it's doing a terrible job at it.
(In reply to Benoit Girard (:BenWa) from comment #51)
> So you're saying this matches with what I saw in the replay? Do we still
> need to file a new bug to improve the semantics of the 'load' event for
> ImageDocument?

Line 596 isn't the load event for the image document, it's the load event for the img element that is inside the image document (the image document being a small html document). Since it is only the load event of the img element we don't expect a style sheet associated with the document to be loaded yet. So what you observed in replay makes sense.

What gets stored in mImageWidth/Height still depends on the ordering of the img element load event and when the CSS property image-orientation gets applied, correct? So that would still be a bug.

Seems like we could easily solve that by calling UpdateSizeFromLayout() in ImageDocument::OnLoadComplete (which is the image document load notification). mImageWidth/Height would be wrong before load, but at least we correct it.
(Assignee)

Comment 53

a year ago
Yes, but it will still be wrong if something causes the image width/height to change. Which AFAIK is pretty much just the inspector changes.
(In reply to Benoit Girard (:BenWa) from comment #53)
> Yes, but it will still be wrong if something causes the image width/height
> to change. Which AFAIK is pretty much just the inspector changes.

Talked to Benoit on irc. We agreed using the devtools to edit the image doc isn't a very important case to worry about.
(In reply to Timothy Nikkel (:tnikkel) from comment #52)
> Seems like we could easily solve that by calling UpdateSizeFromLayout() in
> ImageDocument::OnLoadComplete (which is the image document load
> notification). mImageWidth/Height would be wrong before load, but at least
> we correct it.

OnLoadComplete is for the load notification for the image coming from imagelib. So it is also not the image document load.
(In reply to Timothy Nikkel (:tnikkel) from comment #55)
> (In reply to Timothy Nikkel (:tnikkel) from comment #52)
> > Seems like we could easily solve that by calling UpdateSizeFromLayout() in
> > ImageDocument::OnLoadComplete (which is the image document load
> > notification). mImageWidth/Height would be wrong before load, but at least
> > we correct it.
> 
> OnLoadComplete is for the load notification for the image coming from
> imagelib. So it is also not the image document load.

Filed bug 1251088 for this.

Comment 57

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/93f722efcdec
Depends on: 1251796
Backed out because bug 1251796 (which this caused) is more severe than this one
https://hg.mozilla.org/integration/mozilla-inbound/rev/34c4f2fb86e9
6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* fx-team: 2

Platform breakdown:
* windows7-32: 6

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2016-02-22&endday=2016-02-28&tree=all
(Assignee)

Comment 60

a year ago
I'm waiting on bug 1251280 to be fixed so that I don't modify stuff on top of broken behavior.

After that I'll look if removing width/height 100% prevents bug 1251796.
Depends on: 1251280
5 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 5

Platform breakdown:
* windows7-32: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2016-03-14&endday=2016-03-20&tree=all
6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-aurora: 3
* fx-team: 2
* mozilla-inbound: 1

Platform breakdown:
* windows7-32: 6

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2016-03-21&endday=2016-03-27&tree=all
7 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* fx-team: 2
* mozilla-aurora: 1

Platform breakdown:
* windows7-32: 7

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2016-03-28&endday=2016-04-03&tree=all
8 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 3
* try: 2
* mozilla-central: 2
* mozilla-aurora: 1

Platform breakdown:
* windows7-32: 7
* osx-10-6: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1145903&startday=2016-04-04&endday=2016-04-10&tree=all
Bug 1251088, which I just landed will probably fix this. It fixes the problem at the document load event. The image will still appear with the wrong aspect ratio before that point (there's a bug for that but I can't find it right now), so something like the patch in this bug is still wanted.
(Assignee)

Comment 66

a year ago
Thanks Timothy!
(Assignee)

Updated

10 months ago
Duplicate of this bug: 1242089
(Assignee)

Updated

10 months ago
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago10 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1251088
You need to log in before you can comment on or make changes to this bug.