Closed
Bug 1158412
Opened 9 years ago
Closed 8 years ago
"Assertion failure: false (Unable to find document prescontext and base URI)" with srcset, adoptNode
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: jruderman, Assigned: edgar, Mentored)
References
Details
(Keywords: assertion, testcase, Whiteboard: [tw-dom] btpp-active)
Attachments
(3 files)
Assertion failure: false (Unable to find document prescontext and base URI), at dom/base/ResponsiveImageSelector.cpp:296
Reporter | ||
Comment 1•9 years ago
|
||
Comment 2•9 years ago
|
||
Nothing ever requires a document to have a prescontext, so asserting that it does is really not ok...
Updated•9 years ago
|
Flags: needinfo?(josh)
Comment 4•9 years ago
|
||
(In reply to Not doing reviews right now from comment #2) > Nothing ever requires a document to have a prescontext, so asserting that it > does is really not ok... This is my fault - The pres context is used below getting the current CSS-to-dev-pixels ratio: > double displayDensity = pctx->CSSPixelsToDevPixels(1.0f); Ideally for responsive content we'd use a best guess (parent/root window?) before just falling back to 1.0. Finding the best image (and what density we choose to compare against) is still left up to the UA in the spec There's a similar use in ComputeFinalWidthForCurrentViewport for app units -> css pixels
Updated•9 years ago
|
Assignee: nobody → josh
Flags: needinfo?(josh)
Updated•8 years ago
|
Assignee: josh → nobody
Mentor: josh
Whiteboard: [tw-dom]
Comment 5•8 years ago
|
||
That particular assertion seems to have been removed, but there's another similar one in ComputeFinalWidthForCurrentViewport.
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Josh Matthews [:jdm] from comment #5) > That particular assertion seems to have been removed, but there's another > similar one in ComputeFinalWidthForCurrentViewport. We won't have chance to hit the assertion in ComputeFinalWidthForCurrentViewport(), because the SelectImage returns earlier before calling into ComputeFinalWidthForCurrentViewport();
Assignee | ||
Comment 7•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → echen
Whiteboard: [tw-dom] → [tw-dom] btpp-active
Assignee | ||
Comment 8•8 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e0297124831ca344d137570fb113f660addaf43
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8741840 [details] [diff] [review] Patch, v1 Review of attachment 8741840 [details] [diff] [review]: ----------------------------------------------------------------- As I mentioned in comment #6, it is safe to remove the assertion in ComputeFinalWidthForCurrentViewport() and I still add a crash test for them.
Attachment #8741840 -
Flags: review?(josh)
Updated•8 years ago
|
Attachment #8741840 -
Flags: review?(josh) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 10•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/038ff8933b43
Keywords: checkin-needed
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/038ff8933b43
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 12•8 years ago
|
||
(In reply to Edgar Chen [:edgar][:echen] from comment #6) > (In reply to Josh Matthews [:jdm] from comment #5) > > That particular assertion seems to have been removed, but there's another > > similar one in ComputeFinalWidthForCurrentViewport. > > We won't have chance to hit the assertion in > ComputeFinalWidthForCurrentViewport(), because the SelectImage returns > earlier before calling into ComputeFinalWidthForCurrentViewport(); per comment 2, is SelectImage() bailing here proper? Seems like there's an edge case bug here where SelectImage() runs without a pres context and we skip loading any image. Ideally we'd select a best-effort-without-dpi image (lowest density? closest to 1.0?) and ensure we fire a DPI-changed callback when that information is available.
Flags: needinfo?(echen)
Assignee | ||
Comment 13•8 years ago
|
||
(In reply to John Schoenick [:johns] from comment #12) > per comment 2, is SelectImage() bailing here proper? Seems like there's an > edge case bug here where SelectImage() runs without a pres context and we > skip loading any image. Ideally we'd select a best-effort-without-dpi image > (lowest density? closest to 1.0?) and ensure we fire a DPI-changed callback > when that information is available. File bug 1266318 for this. Thank you, John.
Flags: needinfo?(echen)
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•