Adding a method on platform side to override DPI per window

RESOLVED FIXED in Firefox 51

Status

P1
normal
RESOLVED FIXED
3 years ago
6 months ago

People

(Reporter: zer0, Assigned: zer0)

Tracking

({dev-doc-needed})

unspecified
Firefox 51
dev-doc-needed
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox51 fixed)

Details

(Whiteboard: [multiviewport] [mvp-rdm])

Attachments

(1 attachment)

Responsive Mode needs the capability to specified a different display density / pixel ratio than the default one.

That can be achieved just using `fullZoom`, but it seems not affecting `mozmm`; for such, we need a method on platform side (see Bug 758011).
Flags: qe-verify?
Flags: qe-verify? → qe-verify-
Priority: -- → P3
Priority: P3 → P2
Whiteboard: [multiviewport] → [multiviewport] [mvp-rdm]
(Assignee)

Updated

3 years ago
Assignee: nobody → zer0
Status: NEW → ASSIGNED
Iteration: --- → 49.2 - May 23
Priority: P2 → P1
Iteration: 49.2 - May 23 → 49.3 - Jun 6
Iteration: 49.3 - Jun 6 → 50.1
(Assignee)

Updated

3 years ago
Summary: Adding a method on platform side to override DPI for mozmm → Adding a method on platform side to override DPI per window
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1241891
(Assignee)

Updated

3 years ago
Blocks: 1254385
Iteration: 50.1 → 50.2
(Assignee)

Comment 2

3 years ago
Created attachment 8764613 [details]
Bug 1241867 - override the DPR without affecting the rendering;

- added overrideDPPX to DOMWindowUtils
- made CSSStyleSheet and GlobalWindow using the overrideDPPX value
- added unit test

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

Comment 3

3 years ago
Sorry for the typo ("affectiong"); going to fix when addressing the review comments!
(Assignee)

Comment 4

3 years ago
Comment on attachment 8764613 [details]
Bug 1241867 - override the DPR without affecting the rendering;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60376/diff/1-2/
Attachment #8764613 - Attachment description: Bug 1241867 - override the DPR without affectiong the rendering; → Bug 1241867 - override the DPR without affecting the rendering;
(Assignee)

Comment 5

3 years ago
Note: There is a dependency in the unit test I added that is not used from that file, I have to remove it.
In addition, I have the editor set to remove trailing space during saving – it's part of the editor's settings for devtools –; Markus if that is a problem I will add again the extra space.
(Assignee)

Updated

3 years ago
Attachment #8764613 - Flags: feedback?(jryans)
https://reviewboard.mozilla.org/r/60376/#review57466

::: dom/interfaces/base/nsIDOMWindowUtils.idl:1034
(Diff revision 2)
>     */
>    readonly attribute float fullZoom;
>  
>    /**
> +   * Override the DPPX for the window, without affecting the rendering, but
> +   * only `window.devicePixelRation` and media queries.

Nit: `devicePixelRatio` not `Ration`
Comment on attachment 8764613 [details]
Bug 1241867 - override the DPR without affecting the rendering;

High level approach seems logical to me, which is all I wanted to check over.  (I am not a reviewer for this area of course!)
Attachment #8764613 - Flags: feedback?(jryans) → feedback+
Comment on attachment 8764613 [details]
Bug 1241867 - override the DPR without affecting the rendering;

https://reviewboard.mozilla.org/r/60376/#review57908

Fixing whitespace is fine, but doing it in the same patch that contains actual functionality changes is not. Please separate out your editor's automatic fix into a separate patch.

This patch looks good to me but somebody else should double-check whether the approach is sound. Please request review from dbaron once you've extracted the whitespace changes.

::: layout/base/nsPresContext.cpp:1300
(Diff revision 2)
>  
>    mSuppressResizeReflow = false;
>  }
>  
> +void
> +nsPresContext::SetOverrideDPPX(float aDPPX) {

opening brace goes on the next line
Attachment #8764613 - Flags: review?(mstange)
(Assignee)

Comment 9

3 years ago
Comment on attachment 8764613 [details]
Bug 1241867 - override the DPR without affecting the rendering;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60376/diff/2-3/
Attachment #8764613 - Flags: feedback+ → review?(mstange)
Comment on attachment 8764613 [details]
Bug 1241867 - override the DPR without affecting the rendering;

Okay, I should have put back all the whitespace for the review, and also fixes the typo and the opening brace.

dbaron, could you please review this patch to check if the approach is good? Basically we want to override the value returned by media queries and devicePixelRatio for devtools purpose: in Responsive Design Mode we want to have the capability to test how a website / app will react with different devices that also have different pixel ratios.
Attachment #8764613 - Flags: review?(mstange) → review?(dbaron)
One thought so far:

This doesn't apply the change to subframes, which it probably should.  To fix that, I'd suggest putting the getter/setter on nsIContentViewer instead of nsIDOMWindowUtils, and using the existing mechanism in nsDocumentViewer.cpp for applying these things recursively to subframes.

I still need to look at who *uses* this number and whether the results will make sense.
Iteration: 50.2 - Jul 4 → 50.3 - Jul 18
Actually, this is going to move faster if I ask you to explain your patch rather than waiting for me to find time to figure it out.

Could you explain what this patch is intended to change, and whether all 5 of:
 * units in CSS media queries
 * web-exposed Window.devicePixelRatio
 * the three internal callers of nsPIDOMWindow::GetDevicePixelRatio in http://searchfox.org/mozilla-central/search?q=getdevicepixelratio&path=
match this intent?
Flags: needinfo?(zer0)
Iteration: 50.3 - Jul 18 → 50.4 - Aug 1
Comment on attachment 8764613 [details]
Bug 1241867 - override the DPR without affecting the rendering;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60376/diff/3-4/
Attachment #8764613 - Flags: review?(dbaron) → review?(mstange)
(Assignee)

Updated

2 years ago
Attachment #8764613 - Flags: review?(dbaron)
(In reply to David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) from comment #11)

> One thought so far:
> 
> This doesn't apply the change to subframes, which it probably should.

Yeah, definitely! Thanks! I applied the changes you suggested, putting the getter and the setter in nsIContentViewer; and then I basically mimic the behavior of fullZoom inside nsDocumentViewer.

> I still need to look at who *uses* this number and whether the results will
> make sense.

> Actually, this is going to move faster if I ask you to explain your patch
> rather than waiting for me to find time to figure it out.

So, as said, we basically want to "fake" the values returned by `devicePixelRatio` and media queries inside the Responsive Design Mode, without affecting the rendering – it means, no fullZoom / css scaling. So we thought that this would be the way to approach the solution. To answer to your points:

>  * units in CSS media queries

I'm not sure what you mean exactly with that; if you're referring to the CSSStyleSheet.cpp code, my understanding was that we're converting any value in DPI (e.g. if the unit measure is in centimeter, then `actualDPI` needs to be multiplied by 2.54); since `overrideDDPX` is meant to be in pixel, the conversion by multiply the value by 96.  
If I missed something or there is a side effect by doing that, please, let me know!

>  * web-exposed Window.devicePixelRatio

yes, that's one of the main goal.

>  * the three internal callers of nsPIDOMWindow::GetDevicePixelRatio in
> http://searchfox.org/mozilla-central/search?q=getdevicepixelratio&path=
> match this intent?

I believe so. The page loaded in the browser needs to be "tricked" to believe that the devicePixelRatio is different from what is actually is; but without modifying the overall rendering – like fullZoom does.
Flags: needinfo?(zer0)
Iteration: 50.4 - Aug 1 → 51.1 - Aug 15
Comment on attachment 8764613 [details]
Bug 1241867 - override the DPR without affecting the rendering;

https://reviewboard.mozilla.org/r/60376/#review67794

r=dbaron with the changes below

::: docshell/base/nsIContentViewer.idl:206
(Diff revision 4)
>    attribute float textZoom;
>  
>    /** The amount by which to scale all lengths. Default is 1.0. */
>    attribute float fullZoom;
>  
> +  /** 

please fix the trailing whitespace here

::: docshell/base/nsIContentViewer.idl:207
(Diff revision 4)
> +   * The value used to override devicePixelRatio and media queries dppx.
> +   * Default is 0.0.

This comment should be clearer that 0.0 (or any negative value) means that no overriding is done.

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:12
(Diff revision 4)
> +<p id="display"></p>
> +

You don't use this, so remove it.

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:22
(Diff revision 4)
> +
> +SimpleTest.waitForExplicitFinish();
> +
> +const frameWindow = document.querySelector("iframe").contentWindow;
> +
> +const docShellFor = (win) => 

trailing whitespace

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:22
(Diff revision 4)
> +const docShellFor = (win) => 
> +  SpecialPowers.wrap(win)
> +    .QueryInterface(SpecialPowers.Ci.nsIInterfaceRequestor)
> +    .getInterface(SpecialPowers.Ci.nsIWebNavigation)
> +    .QueryInterface(SpecialPowers.Ci.nsIDocShell);
> +
> +const docShell = docShellFor(window);
> +const frameDocShell = docShellFor(frameWindow);

Rather than writing all of this, I think it's preferable to add getOverrideDPPX and setOverrideDPPX to testing/specialpowers/content/specialpowersAPI.js, and then just use SpecialPowers.setOverrideDPPX(win, ...), etc.

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:32
(Diff revision 4)
> +
> +const docShell = docShellFor(window);
> +const frameDocShell = docShellFor(frameWindow);
> +
> +const originalDPR = window.devicePixelRatio;
> +const originalZoom = docShell.contentViewer.fullZoom;

use SpecialPowers.getFullZoom(frameWindow)

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:36
(Diff revision 4)
> +const originalDPR = window.devicePixelRatio;
> +const originalZoom = docShell.contentViewer.fullZoom;
> +const dppx = originalDPR * 1.5;
> +const zoom = originalZoom * 0.5;
> +
> +const overrideDPPX = (value) => {

Probably clearer to call these SetOverrideDPPX and SetFullZoom.

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:65
(Diff revision 4)
> +  style.sheet.insertRule(`body {font-size: ${size}px}`, 0);
> +
> +  return style;
> +}
> +
> +const ensureIntegrityInitialValues = () => {

How about "assertValuesAreInitial" instead?

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:110
(Diff revision 4)
> +    is(frameWindow.devicePixelRatio, dppx,
> +      "frame's devicePixelRatio overridden, fullZoom ignored");
> +
> +    overrideDPPX(0);
> +
> +    is(window.devicePixelRatio, originalDPR * zoom,

maybe also assert here that:

isnot(dppx, originalDPR * zoom,
      "test is no longer testing what it should be");

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:131
(Diff revision 4)
> +    ensureIntegrityInitialValues();
> +
> +    let frameDoc = frameWindow.document;
> +
> +    let originalFontSize = getComputedStyle(document.body).fontSize;
> +    let frameOriginalFontSize = getComputedStyle(frameDoc.body).fontSize;

please use frameWindow.getComputedStyle() rather than just getComputedStyle() here

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:137
(Diff revision 4)
> +
> +    let style = createFontStyleForDPPX(document, dppx, "32");
> +    let frameStyle = createFontStyleForDPPX(frameDoc, dppx, "32");
> +
> +    let currentFontSize = getComputedStyle(document.body).fontSize;
> +    let frameCurrentFontSize = getComputedStyle(frameDoc.body).fontSize;

please use frameWindow.getComputedStyle() rather than just getComputedStyle() here

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:140
(Diff revision 4)
> +
> +    let currentFontSize = getComputedStyle(document.body).fontSize;
> +    let frameCurrentFontSize = getComputedStyle(frameDoc.body).fontSize;
> +
> +    is(currentFontSize, originalFontSize,
> +      "media query are not applied yet");

"media query are" should be "media query is" (or "media queries are"), throughout

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:147
(Diff revision 4)
> +      "frame's media query are not applied yet");
> +
> +    overrideDPPX(dppx);
> +
> +    currentFontSize = getComputedStyle(document.body).fontSize;
> +    frameCurrentFontSize = getComputedStyle(frameDoc.body).fontSize;

please use frameWindow.getComputedStyle() rather than just getComputedStyle() here

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:162
(Diff revision 4)
> +      "frame's font size has the expected value.");
> +
> +    overrideDPPX(0);
> +
> +    currentFontSize = getComputedStyle(document.body).fontSize;
> +    frameCurrentFontSize = getComputedStyle(frameDoc.body).fontSize;

please use frameWindow.getComputedStyle() rather than just getComputedStyle() here

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:180
(Diff revision 4)
> +    ensureIntegrityInitialValues();
> +
> +    let frameDoc = frameWindow.document;
> +
> +    let styles = [
> +      createFontStyleForDPPX(document, originalDPR, "16"),

Since 16px is the initial value of font-size, you should probably use something more unusual here (say, 23px?).  Same 3 lines below.

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:189
(Diff revision 4)
> +      createFontStyleForDPPX(frameDoc, dppx, "32"),
> +      createFontStyleForDPPX(frameDoc, originalDPR * zoom, "48")
> +    ];
> +
> +    let currentFontSize = getComputedStyle(document.body).fontSize;
> +    let frameCurrentFontSize = getComputedStyle(frameDoc.body).fontSize;

please use frameWindow.getComputedStyle() rather than just getComputedStyle() here, and the rest of the relevant places in this function

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:201
(Diff revision 4)
> +    overrideDPPX(dppx);
> +
> +    currentFontSize = getComputedStyle(document.body).fontSize;
> +    frameCurrentFontSize = getComputedStyle(frameDoc.body).fontSize;
> +    is(currentFontSize, "32px",
> +      "media query are applied for overridden DDPX, fullZoom ignored.");

; rather than , in the message here (and 2 lines below)

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:230
(Diff revision 4)
> +    let promises = [
> +      new Promise(resolve => {
> +        let mql = window.matchMedia(`(resolution: ${dppx}dppx)`);
> +
> +        mql.addListener(function listener() {
> +          ok("MediaQueryList's listener invoked.")
> +          mql.removeListener(listener);
> +          resolve();
> +        });
> +      }),
> +      new Promise(resolve => {
> +        let mql = frameWindow.matchMedia(`(resolution: ${dppx}dppx)`);
> +
> +        mql.addListener(function listener() {
> +          ok("frame's MediaQueryList's listener invoked.")
> +          mql.removeListener(listener);
> +          resolve();
> +        });
> +      })
> +    ];
> +
> +    Promise.all(promises)
> +      .then(() => overrideDPPX(0))
> +      .then(done, e => {throw e});

You need something here to ensure that these listeners actually get called, i.e., a test assertion that will fail if they don't.

(It's actually not clear to me that they will, since I think the listener will be added after the change happens, and I think you need to fix that.)

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:298
(Diff revision 4)
> +  }
> +
> +  SimpleTest.finish();
> +};
> +
> +const gTestRunner = runner(gTests);

I don't quite follow how this is working.  Doesn't this go ahead an execute the first test immediately, so you then end up with one sequence of tests that was triggered here and (potentially) another sequence triggered from the load event?

Could you explain?

::: dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html:304
(Diff revision 4)
> +    } catch (e) {
> +      if (e !== StopIteration) { throw e; }
> +    }

Does this ever even happen, given that you call SimpleTest.finish() before the generator completes?  (Though I tend to think that's probably not the best idea, and maybe you should call SimpleTest.finish() when the iteration completes.)

::: layout/base/nsDocumentViewer.cpp:359
(Diff revision 4)
>    nsCOMPtr<nsIContentViewer> mPreviousViewer;
>    nsCOMPtr<nsISHEntry> mSHEntry;
>  
>    nsIWidget* mParentWidget; // purposely won't be ref counted.  May be null
>    bool mAttachedToParent; // view is attached to the parent widget
>  

(commenting on a random line)

There's an additional caller of SetOverrideDPPX that I think you're missing, which is in the function TransferZoomLevels in dom/base/nsDocument.cpp, which is needed to transfer the override to external resource documents.

::: layout/base/nsDocumentViewer.cpp:3101
(Diff revision 4)
> +  // Check the prescontext first because it might have a temporary
> +  // setting for print-preview

Are you confident that this is actually true, and we do temporarily end up with a different value for print preview?

::: layout/base/nsPresContext.cpp:1306
(Diff revision 4)
> +{
> +  mOverrideDPPX = aDPPX;
> +
> +  if (HasCachedStyleData()) {
> +    // All cached style data must be recomputed.
> +    MediaFeatureValuesChanged(eRestyle_ForceDescendants, NS_STYLE_HINT_REFLOW);

I don't see why cached style data are invalid.  Isn't the only thing that changes here the results of media queries?

If that's the case, then you should use:

MediaFeatureValuesChanged(nsRestyleHint(0), nsChangeHint(0)).

See the comment above the declaration of nsPresContext::MediaFeatureValuesChanged.



If you need more than that, I'd like to specifically understand why.  (In other words, if you can't make this exact change, then I retract the r=dbaron until I understand it.)
Attachment #8764613 - Flags: review?(dbaron) → review+
Matteo, I don't think this requires another review from me.
Iteration: 51.1 - Aug 15 → 51.2 - Aug 29
Attachment #8764613 - Flags: review?(mstange)
(Assignee)

Comment 17

2 years ago
mozreview-review-reply
Comment on attachment 8764613 [details]
Bug 1241867 - override the DPR without affecting the rendering;

https://reviewboard.mozilla.org/r/60376/#review67794

> You need something here to ensure that these listeners actually get called, i.e., a test assertion that will fail if they don't.
> 
> (It's actually not clear to me that they will, since I think the listener will be added after the change happens, and I think you need to fix that.)

This part rely on the test's timeout for failing, as usually we do when we deal with listeners – since the only way to fail is when they're not called, but since they're asyncronous we can't determined when they will be called.
So, I can add a manual timeout, here (using `setTimeout`) but I will basically reproduce what the framework's test already does.

(The listeners are added before the change happens, because they're added in the `executor` function of the promise, so they're already added when we'll invoke `overrideDPPX(dppx)` on line 255)

> I don't quite follow how this is working.  Doesn't this go ahead an execute the first test immediately, so you then end up with one sequence of tests that was triggered here and (potentially) another sequence triggered from the load event?
> 
> Could you explain?

This line basically creates a `Generator` object, since `runner` contains `yield`. I basically evolved a bit what we already have in similar tests (however, I missed that `runner` was a "legacy generator", I fixed that in the coming patch adding `function*` instead).

So, `gTestRunner` is a `Generator` object, therefore no code of `runner` is executed yet. The first execution is happening when we'll call the `next()` method of `gTestRunner`, so only one sequence is executed – and it's triggered by the load event.

> Does this ever even happen, given that you call SimpleTest.finish() before the generator completes?  (Though I tend to think that's probably not the best idea, and maybe you should call SimpleTest.finish() when the iteration completes.)

This part is now obsolete since I removed the legacy generator; now it calls `SimpleTest.finish()` when the iteration completes (when the `done` property of the object returned by `next`'s method is `true`); so no `try` / `catch` anymore.

> (commenting on a random line)
> 
> There's an additional caller of SetOverrideDPPX that I think you're missing, which is in the function TransferZoomLevels in dom/base/nsDocument.cpp, which is needed to transfer the override to external resource documents.

Fixed! Thanks for to spot this out! Is there a good way to test it, in case, or it's okay leave the unit test as is?

> Are you confident that this is actually true, and we do temporarily end up with a different value for print preview?

No, I actually just copied the block from full zoom; I think it's just a left over. I removed the comment.

> I don't see why cached style data are invalid.  Isn't the only thing that changes here the results of media queries?
> 
> If that's the case, then you should use:
> 
> MediaFeatureValuesChanged(nsRestyleHint(0), nsChangeHint(0)).
> 
> See the comment above the declaration of nsPresContext::MediaFeatureValuesChanged.
> 
> 
> 
> If you need more than that, I'd like to specifically understand why.  (In other words, if you can't make this exact change, then I retract the r=dbaron until I understand it.)

You're right, we had an issue about the media queries values not be updated and I copied that from full zoom I guess; the line you suggested works as well! Thanks.
I modified the unit test since an issue on some debug build (I filed bug 1297714 about that).

Here the try build; the orange doesn't seems related to this patch:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7c54f4631df&selectedJob=26337110
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 19

2 years ago
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/125b052639cb
override the DPR without affecting the rendering. r=dbaron
Keywords: checkin-needed
Backed out  for assertion in test_contentViewer_overrideDPPX.html:

https://hg.mozilla.org/integration/fx-team/rev/b02228e2a9eb

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=125b052639cb1879d4a593c715d244c551129118
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=11327888&repo=fx-team

19:18:46     INFO -  [2801] ###!!! ASSERTION: two zooms happening at the same time? impossible!: '!mSuppressResizeReflow', file /home/worker/workspace/build/src/layout/base/nsPresContext.cpp, line 1298
19:19:47     INFO -  #01: nsDocumentViewer::SetFullZoom [layout/base/nsDocumentViewer.cpp:3045]
19:19:47     INFO -  #02: NS_InvokeByIndex [xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:184]
19:19:48     INFO -  #03: CallMethodHelper::Call [js/xpconnect/src/xpcprivate.h:744]
19:19:48     INFO -  #04: XPCWrappedNative::CallMethod [js/xpconnect/src/XPCWrappedNative.cpp:1351]
19:19:48     INFO -  #05: XPC_WN_GetterSetter [js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1172]
19:19:48     INFO -  #06: js::CallJSNative [js/src/jscntxtinlines.h:236]
19:19:48     INFO -  #07: js::InternalCallOrConstruct [js/src/vm/Interpreter.cpp:454]
19:19:48     INFO -  #08: js::Call [js/src/vm/Interpreter.cpp:518]
19:19:48     INFO -  #09: js::CallSetter [js/src/vm/Interpreter.cpp:644]
19:19:48     INFO -  #10: js::NativeSetProperty [js/src/vm/NativeObject.cpp:2363]
19:19:48     INFO -  #11: Reflect_set [js/src/vm/NativeObject.h:1496]
19:19:48     INFO -  #12: ??? (???:???)
19:19:48     INFO -  MEMORY STAT | vsize 1061MB | residentFast 247MB | heapAllocated 97MB
19:19:48     INFO -  3248 INFO TEST-OK | dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html | took 844ms
19:19:48     INFO -  ++DOMWINDOW == 60 (0x7f10ee1cfc00) [pid = 2801] [serial = 91] [outer = 0x7f10eaf50000]
19:19:48     INFO -  3249 INFO TEST-UNEXPECTED-ERROR | dom/tests/mochitest/general/test_contentViewer_overrideDPPX.html | Assertion count 1 is greater than expected range 0-0 assertions.
Flags: needinfo?(zer0)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
yeah, I noticed that I didn't update the commit after I filed bug 1241867; the try build actually had the correct fix.
I tried to push the new ones on mozreview, but it there are some issues probably due the rebasing with master – also I noticed some whitespace that shouldn't be there.
Since it's a bit late here, I'll fix them tomorrow morning. My apologies!
Flags: needinfo?(zer0)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 25

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/929f5a625e2e
override the DPR without affecting the rendering; r=dbaron
Keywords: checkin-needed
Iteration: 51.2 - Aug 29 → 51.3 - Sep 12

Comment 26

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/929f5a625e2e
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
The IDL change should be described at https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIContentViewer#Attributes.

Sebastian
Keywords: dev-doc-needed

Updated

6 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.