Render the HTML preview within the Response side-panel

RESOLVED FIXED in Firefox 59

Status

RESOLVED FIXED
2 years ago
4 months ago

People

(Reporter: Honza, Assigned: brandon.cheng)

Tracking

({dev-doc-complete, good-first-bug})

unspecified
Firefox 59
dev-doc-complete, good-first-bug

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(5 attachments)

(Reporter)

Description

2 years ago
Follow up for bug 1350229

HTML preview was previously implemented as a side panel and removed in bug 1350229.

We should re-introduce this feature and render the preview within the existing Response side panel.

Honza
(Reporter)

Updated

2 years ago
Blocks: 1348737
Flags: qe-verify?
Whiteboard: [devtools-html] [triage]

Updated

2 years ago
Whiteboard: [devtools-html] [triage] → [netmonitor] [triage]
(Reporter)

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → INVALID

Updated

2 years ago
No longer blocks: 1348737
Flags: qe-verify?
Whiteboard: [netmonitor] [triage]
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Still valid according to the last comment in bug 1247392
Keywords: good-first-bug
(Assignee)

Comment 2

a year ago
Hi. May I be assigned this bug?
Assignee: nobody → brandon.cheng
Hey Brandon, have you made progress on this bug ?

Let us know if you need any help.
Flags: needinfo?(brandon.cheng)
(Assignee)

Comment 4

a year ago
Hi Tim!

Thanks for checking in. I have an initial version that simply restores the previous functionality in the Response panel, but it replaces the "Response payload" component that's currently there. I saw that the latest Nightly changed the behavior of JSON previews to show both the "PropertiesView" component and the raw "Response payload". (It previously showed just the PropertiesView.)

http://imgur.com/a/0DF13

I'd like for HTML previews to behave the same and show both the preview and the raw response payload, so I'm working on that at the moment. I'm currently trying to understand more of the netmonitor codebase so my patch doesn't disrupt or change too much, but I do have a question that would help me do this faster.

Assuming that mimicing the JSON tree view behavior would be desired, would it make sense to extend PropertiesView to show an arbitrary component (the iframe in this case), or use something other than the PropertiesView when showing an HTML preview?

Thank you!
Flags: needinfo?(brandon.cheng)
Hi Brandon, thanks for the update!

(In reply to Brandon Cheng from comment #4)
> I'd like for HTML previews to behave the same and show both the preview and
> the raw response payload
Yes, this is the expected behaviour.

> Assuming that mimicing the JSON tree view behavior would be desired, would
> it make sense to extend PropertiesView to show an arbitrary component (the
> iframe in this case), or use something other than the PropertiesView when
> showing an HTML preview?

You could rename renderRowWithEditor [0] to renderRowWithExtras then make it support iframe previews, or more generic components if that's not too complicated. The function shows how we add support for rendering Editors in the property tree.

[0]: https://dxr.mozilla.org/mozilla-central/source/devtools/client/netmonitor/src/components/properties-view.js#86
Duplicate of this bug: 1393730
Duplicate of this bug: 1372949
Depends on: 1350229

Comment 8

a year ago
With Firefox 55 hiting release, many more users are being affected by this. This is a essential functionality for front-end and PHP developers. Some even had to move an entire development team to chrome because of this! If this could get some priority, please prioritize it!

See: https://www.reddit.com/r/firefox/comments/6h7n50/why_was_http_response_preview_removed_from/
(Assignee)

Comment 9

a year ago
(In reply to Danilo from comment #8)
> This is a essential functionality for front-end and PHP developers.

I fully agree. I'll do my best to get the initial patch in by tomorrow morning. Let me note quickly the patch will have to through review and won't hit the stable release until Firefox 57.
In case the review is positive and this lands the next days, maybe it could be requested to uplift the patch to 56?

Sebastian
Status: REOPENED → ASSIGNED
(Assignee)

Comment 11

a year ago
Here's an update. With Tim's help I was able to get the HTML preview to show alongside the "Response payload" panel. I'm now tackling an issue with the .editor-row-container being larger than it's supposed to be. It has a CSS height of 100%, causing the container (or response panel) to overflow. This isn't too noticable on the current latest Nightly, but you can see how the horizontal scroll bar is hidden in an unscrollable area when panning left and right. The response payload label takes up the space that the horizontal scrollbar would normally be. Adding the HTML preview exacerbated this in that now parts of the raw payload are unreachable.

I thought about filing this as a separate bug, but I'll try to fix it here first. I can definitely file a bug with a more detailed report and a video if someone would like though.

I think a vertical flexbox container would solve this well. Going to try and tackle that today; I'll submit the patch as soon as that works.
(Assignee)

Comment 12

a year ago
Haha, problem solved! Turns out the container was already a vertical flexbox.
Thanks for the hard work Brandon! Feel free to upload the patch and ask for feedback on this CSS issue. It sounds like you might want a overflow: hidden or auto depending on the use case. Using a vertical flexbox or flexbox in general is usually a good idea.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 16

a year ago
(In reply to Gabriel [:gl] (ΦωΦ) from comment #13)
> Thanks for the hard work Brandon! Feel free to upload the patch and ask for
> feedback on this CSS issue.

Thank to you guys for putting letting us put this feature back in! Commits should be uploaded now.

I have to say, ReviewBoard is a breath of fresh air from the old patch system.
Thanks again for the patch. We will try to get this reviewed asap.

Comment 18

a year ago
mozreview-review
Comment on attachment 8903233 [details]
Bug 1353319 - Make SourceEditor flex when next to an HTML preview.

https://reviewboard.mozilla.org/r/175026/#review180232

Thanks for working on this! The UI looks fine to me.

::: devtools/client/netmonitor/src/assets/styles/netmonitor.css:1041
(Diff revision 1)
> +
> +.html-preview {
> +  height: 100%;
> +}
> +
> +.html-preview iframe {

Can you add a white background to the iframe ? That would be a sensible default for an iframe IMO, especially for the dark theme.

::: devtools/client/netmonitor/src/components/properties-view.js:115
(Diff revision 1)
> +    if (level >= 1 && (path.includes(EDITOR_CONFIG_ID) ||
> +      path.includes(HTML_PREVIEW_ID))) {

nit: I feel like this would be easier to read:

if ((path.includes(EDITOR_CONFIG_ID) || path.includes(HTML_PREVIEW_ID))
    && level >= 1) {

::: devtools/client/netmonitor/src/components/response-panel.js:25
(Diff revision 1)
>  const JSON_SCOPE_NAME = L10N.getStr("jsonScopeName");
>  const JSON_FILTER_TEXT = L10N.getStr("jsonFilterText");
>  const RESPONSE_IMG_NAME = L10N.getStr("netmonitor.response.name");
>  const RESPONSE_IMG_DIMENSIONS = L10N.getStr("netmonitor.response.dimensions");
>  const RESPONSE_IMG_MIMETYPE = L10N.getStr("netmonitor.response.mime");
> +const RESPONSE_PREVIEW = L10N.getStr("responsePreview");

nit: move RESPONSE_PREVIEW after RESPONSE_PAYLOAD (alphabetical ordering)

::: devtools/client/netmonitor/src/components/response-panel.js:113
(Diff revision 1)
> +  // Detecting HTML isn't as simple as detecting JSON since problematic
> +  // HTML may still render fine. We'll do our best and go off of the MIME type.
> +  isHTML(mimeType) {
> +    return /text\/html/.test(mimeType);
> +  },

I wonder why this is needed, since the original implementation of the HTML Preview panel uses:

```
const { Filters } = require("../utils/filter-predicates");
...
Filters.html(this.props.request);
```
Attachment #8903233 - Flags: review?(ntim.bugs)

Comment 19

a year ago
mozreview-review
Comment on attachment 8903232 [details]
Bug 1353319 - Render HTML preview within Response side-panel.

https://reviewboard.mozilla.org/r/175024/#review180248

::: commit-message-fb224:1
(Diff revision 1)
> +Bug 1353319 - Fix SourceEditor component overflow; r?ntim

Seems like your change breaks the ability to see the raw JSON response for long JSON files:

Example: http://api.duckduckgo.com/?q=valley+forge+national+park&format=json&pretty=1
Attachment #8903232 - Flags: review?(ntim.bugs)
Created attachment 8903412 [details]
Screen Shot 2017-09-01 at 9.09.10 AM.png

Here's the JSON issue I'm talking about, the "Response Payload" section isn't expandable.
(Assignee)

Comment 21

a year ago
(In reply to Tim Nguyen :ntim from comment #19)
> Comment on attachment 8903232 [details]
> Bug 1353319 - Fix SourceEditor component overflow;
> 
> https://reviewboard.mozilla.org/r/175024/#review180248
> 
> ::: commit-message-fb224:1
> (Diff revision 1)
> > +Bug 1353319 - Fix SourceEditor component overflow; r?ntim
> 
> Seems like your change breaks the ability to see the raw JSON response for
> long JSON files:
> 
> Example:
> http://api.duckduckgo.com/?q=valley+forge+national+park&format=json&pretty=1

That's quite bad. Apologies, I got too excited at fixing the bottom horizontal scroll bar and didn't test well enough.

So I see three ways that the SourceEditor component can behave while in the Response panel.

1. Have a container with "height: 100%;", and scrolling within the SourceEditor layer. This is what we do now, but is problematic since you have two scroll contexes and it's hard to reach the end of the SourceEditor. At least on macOS, the container's scrollbar and the SourceEditor's scrollbar are hidden by default, so you have to awkwardly scroll on the labels to get around in some cases. I'll attach a video showing this.
2. Have a container with "flex-grow: 1;" and share space with another flex item. This is what I thought would work well, but doesn't work for the long JSON case you showed me. It's obvious to me now why this didn't work, since the remaining flexible area to grow is 0 in this case.
3. Have a container with "height: auto;" and only 1 scroll context on the container. I would like to be given the okay to explore this since it would work well for SourceEditor's current usage and here. I also think 1 scroll context is better usability wise.

To do #3 though, we have to remove the mouse wheel listener on the Editor component. It looks like we're currently capturing it in order to update the line numbers gutter position.

https://dxr.mozilla.org/mozilla-central/source/devtools/client/sourceeditor/editor.js#338
(Assignee)

Comment 22

a year ago
Here's a video of the problem with #1.

https://cloudup.com/cKaJ0c3IKYE
Flags: needinfo?(ntim.bugs)
See Also: → bug 1247392
Honza, could you take a look at this ?
Flags: needinfo?(ntim.bugs) → needinfo?(odvarko)
This seems close to be landable, it would be great to try and have it for Firefox 57!

Brandon: maybe we could simply update your current patch and set `min-height: 50%;` to the selector `.tree-container .treeTable tr:last-child.editor-row-container`? 
This way the editor will still be visible for long JSON responses. 
We can create a separate bug to fix the remaining issues you raised.
Flags: needinfo?(brandon.cheng)
(Reporter)

Comment 25

a year ago
(In reply to Brandon Cheng from comment #21)
> 3. Have a container with "height: auto;" and only 1 scroll context on the
> container. I would like to be given the okay to explore this since it would
> work well for SourceEditor's current usage and here. I also think 1 scroll
> context is better usability wise.
Yes, agree this sounds promising.

However, from longer-term perspective, I'd vote for using an accordion
component e.g. share the one used by Debugger tool. We might also learn from
how Firebug did this. Some analysis needed.

In any case, I tried suggestion from Julian (comment #24) and it looks like
good enough workaround for now. If you do it, I can R+ both patches and
we can land yet in this cycle. The rest of the work can be done in a follow up.

Thanks for working on this!
Honza
Flags: needinfo?(odvarko)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 28

a year ago
(In reply to Jan Honza Odvarko [:Honza] from comment #25)
> In any case, I tried suggestion from Julian (comment #24) and it looks like
> good enough workaround for now. If you do it, I can R+ both patches and
> we can land yet in this cycle. The rest of the work can be done in a follow
> up.
> 
> Thanks for working on this!
> Honza

Thanks for the response Honza. Julian, I appreciate your help as well.

I just pushed something else I've been working on the last few days. I think it's a bit better UI-wise than setting "min-height: 50%;", but not as clean since it relies on a conditional CSS class.

If this alternative isn't ideal, I have no qualms with removing that workaround going Julian's solution.
Flags: needinfo?(brandon.cheng)
(Reporter)

Comment 29

a year ago
mozreview-review
Comment on attachment 8903232 [details]
Bug 1353319 - Render HTML preview within Response side-panel.

https://reviewboard.mozilla.org/r/175024/#review184918

Thanks for working on this!

I am seeing some double-scrollbar issues, but I don't think that there is much we can do about that given the architecture of the PropertiesView. I think that the situation can become a lot easier if we adopt Acordion for all Net side panels.

R+ assuming try is green

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=88f028b37c33fb9003787e331926f35fef5ec0c8

Please, file a follow up bug related to the double scrollbar issues. It might be fixed by using the Acordion, but it shouldn't be forgotten what these issues are.

Honza
Attachment #8903232 - Flags: review?(odvarko) → review+
(Reporter)

Comment 30

a year ago
mozreview-review
Comment on attachment 8903233 [details]
Bug 1353319 - Make SourceEditor flex when next to an HTML preview.

https://reviewboard.mozilla.org/r/175026/#review184922
Attachment #8903233 - Flags: review?(odvarko) → review+
(Reporter)

Comment 31

a year ago
Created attachment 8908013 [details]
response-payload.png

Here is another case where I am seeing double scroll bar (Win10)

It's there if the response payload is bigger than the available area. The second scrollbar is there because the Response Payload section header is not included in 100% height and causes overflow.

Should be mentioned in the follow up report.

Honza
(Reporter)

Comment 32

a year ago
I am seeing this test failure:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=88f028b37c33fb9003787e331926f35fef5ec0c8&selectedJob=130990653

251 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_cyrillic-02.js | The text shown in the source editor is correct. -


Honza
(Reporter)

Updated

a year ago
Duplicate of this bug: 1375063
(Assignee)

Comment 34

a year ago
(In reply to Jan Honza Odvarko [:Honza] from comment #32)
> I am seeing this test failure:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=88f028b37c33fb9003787e331926f35fef5ec0c8&selectedJob=1
> 30990653
> 
> 251 INFO TEST-UNEXPECTED-FAIL |
> devtools/client/netmonitor/test/browser_net_cyrillic-02.js | The text shown
> in the source editor is correct. -
> 
> 
> Honza

I've spent a few days tackling this test and can't seem to get it passing, so I think I have to ask for technical help on this one.

The test can't find the cyrillic text since the HTML preview pushes the source code preview down. And the SourceEditor uses a virtual DOM to only load what's visible. I'm trying to scroll down the source editor a few lines to load more lines into the real DOM, but can't seem to get it working with WheelEvent.

const event = {
  deltaMode: WheelEvent.DOM_DELTA_PAGE,
  deltaY: 1.0,
  lineOrPageDeltaY: 1
}
yield EventUtils.synthesizeWheel(
  document.querySelector(".CodeMirror-scroll"),
  10,
  10,
  event
);
```

Scrolling down without using EventUtils seems to work and get the test working.

document.querySelector(".CodeMirror-scroll").scrollBy(0, 200);

But I imagine that we should be using the mock events instead while testing. I'm still toying around with a few things to try and get the EventUtil event working, but I'm not sure why it's not scrolling this specific component yet.
Flags: needinfo?(odvarko)
(Reporter)

Comment 35

a year ago
@Ricky: any tips what could be wrong here?
Honza
Flags: needinfo?(odvarko) → needinfo?(rchien)
I this test is not asserting the HTML preview, some suggestions:
- collapse the preview pane
- or increase the devtools toolbox height
(In reply to Brandon Cheng from comment #34)
> (In reply to Jan Honza Odvarko [:Honza] from comment #32)
> > I am seeing this test failure:
> > https://treeherder.mozilla.org/#/
> > jobs?repo=try&revision=88f028b37c33fb9003787e331926f35fef5ec0c8&selectedJob=1
> > 30990653
> > 
> > 251 INFO TEST-UNEXPECTED-FAIL |
> > devtools/client/netmonitor/test/browser_net_cyrillic-02.js | The text shown
> > in the source editor is correct. -
> > 
> > 
> > Honza
> 
> I've spent a few days tackling this test and can't seem to get it passing,
> so I think I have to ask for technical help on this one.
> 
> The test can't find the cyrillic text since the HTML preview pushes the
> source code preview down. And the SourceEditor uses a virtual DOM to only
> load what's visible. I'm trying to scroll down the source editor a few lines
> to load more lines into the real DOM, but can't seem to get it working with
> WheelEvent.
> 
> const event = {
>   deltaMode: WheelEvent.DOM_DELTA_PAGE,
>   deltaY: 1.0,
>   lineOrPageDeltaY: 1
> }
> yield EventUtils.synthesizeWheel(
>   document.querySelector(".CodeMirror-scroll"),
>   10,
>   10,
>   event
> );
> ```

What you can try is synthesizing the mousewheel event, then using waitForDOM to wait for the editor to load.

You might also want to double check if the editor is properly expanded (the element that's expanded in the test might change with the new HTML preview).

See this test: https://dxr.mozilla.org/mozilla-central/source/devtools/client/netmonitor/test/browser_net_jsonp.js#71
Flags: needinfo?(brandon.cheng)
Alternatively, you can just use `node.scrollTop` to scroll to the destination you want, and then remember to use waitForDOM for waiting the text.

waitUntil() [1] is another good method of polling a given condition through gecko thread mechanism. I'm not sure whether the position of cyrillic text is predictable. If it's not, waitUntil() might be your best friend to help you keep scrolling the editor content down and polling the text until it appears.

[1] http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/test/browser_net_image-tooltip.js#85
Flags: needinfo?(rchien)

Updated

a year ago
Duplicate of this bug: 1247392

Comment 40

a year ago
Any update on this issue?
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 44

a year ago
mozreview-review-reply
Comment on attachment 8903233 [details]
Bug 1353319 - Make SourceEditor flex when next to an HTML preview.

https://reviewboard.mozilla.org/r/175026/#review180232

Thank you for reviewing this and your help! I'll push the changes as soon as my build finishes and I test that they work.

> Can you add a white background to the iframe ? That would be a sensible default for an iframe IMO, especially for the dark theme.

Makes sense.

> nit: I feel like this would be easier to read:
> 
> if ((path.includes(EDITOR_CONFIG_ID) || path.includes(HTML_PREVIEW_ID))
>     && level >= 1) {

I agree, thanks!

> I wonder why this is needed, since the original implementation of the HTML Preview panel uses:
> 
> ```
> const { Filters } = require("../utils/filter-predicates");
> ...
> Filters.html(this.props.request);
> ```

Ah thank you. It looks like I didn't look at the patch removing the original implementation carefully enough.
(Assignee)

Comment 45

a year ago
I think my response to Tim's initial review didn't go through until I hit publish on ReviewBoard just now. So the above comment was from a while ago.

I just pushed a new commit that grabs CodeMirror's content through a method the library provides. It fixes the test by comparing against buffered content in memory rather than content rendered to the DOM. The commit description contains more details about other things I tried, like EventUtils.sendWheelAndPaint and scrollBy.

I also amended the first two commits so my patches are based off of the latest nightly.

(In reply to Ricky Chien [:rickychien] from comment #38)
> Alternatively, you can just use `node.scrollTop` to scroll to the
> destination you want, and then remember to use waitForDOM for waiting the
> text.
> 
> waitUntil() [1] is another good method of polling a given condition through
> gecko thread mechanism. I'm not sure whether the position of cyrillic text
> is predictable. If it's not, waitUntil() might be your best friend to help
> you keep scrolling the editor content down and polling the text until it
> appears.

This was a good suggestion, but I don't there's an easy way to tell when the cyrillic text is rendered. It's predictably about 200px down, but I'm hesitant to hard-code that into the test.

(In reply to Tim Nguyen :ntim from comment #37)
> You might also want to double check if the editor is properly expanded (the
> element that's expanded in the test might change with the new HTML preview).
> 
> See this test:
> https://dxr.mozilla.org/mozilla-central/source/devtools/client/netmonitor/
> test/browser_net_jsonp.js#71

I added a sleep in the middle of the test to observe what was happening. The editor was properly expanded, just shorter than it was before I added the preview. Waiting for ".CodeMirror-code" didn't work since CodeMirror seems to only add lines when they become visible after scroll. I arrived at this conclusion by getting the value of document.querySelector(".CodeMirror-code").textContent in the browser toolbox minutes after loading the editor. This makes sense to me as libraries like react-virtualized use this as a performance optimization.
Flags: needinfo?(brandon.cheng)

Comment 46

a year ago
mozreview-review
Comment on attachment 8903232 [details]
Bug 1353319 - Render HTML preview within Response side-panel.

https://reviewboard.mozilla.org/r/175024/#review199262

::: devtools/client/netmonitor/src/components/PropertiesView.js:127
(Diff revision 3)
> +        )
> +      );
> +    }
> +
> +    // Skip for editor config and HTML previews
> +    if (path.includes(EDITOR_CONFIG_ID) || path.includes(HTML_PREVIEW_ID)

nit: add brackets () around `path.includes(EDITOR_CONFIG_ID) || path.includes(HTML_PREVIEW_ID)` to avoid ambiguity
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 50

a year ago
(In reply to Jan Honza Odvarko [:Honza] from comment #29)
> Please, file a follow up bug related to the double scrollbar issues. It
> might be fixed by using the Acordion, but it shouldn't be forgotten what
> these issues are.
> 
> Honza

Filed as bug 1412607.
(Reporter)

Comment 51

a year ago
mozreview-review
Comment on attachment 8923134 [details]
Bug 1353319 - Fix cyrillic text test after re-adding the HTML preview.

https://reviewboard.mozilla.org/r/194324/#review200434

Looks good to me!

R+

Push to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bfc599cadf216781f5093fc9f7eb7dea39a827d0&selectedJob=141091153

There is one failure, but I don't know how it could be related.

Honza
Attachment #8923134 - Flags: review?(odvarko) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 55

11 months ago
(In reply to Jan Honza Odvarko [:Honza] from comment #51)
> There is one failure, but I don't know how it could be related.

That test seems to be passing when I run it locally. On a separate note, I just rebased, resolved some merge conflicts, and pushed to mozreview. Seems to still pass on my local machine after rebasing.

Is there anything additional I should do to get this merged into mozilla-central? Or should we investigate that failed debugger test further?
Keywords: checkin-needed

Comment 56

11 months ago
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4bab59d6b58f
Render HTML preview within Response side-panel. r=Honza
https://hg.mozilla.org/integration/autoland/rev/edbc03f93f6a
Make SourceEditor flex when next to an HTML preview. r=Honza
https://hg.mozilla.org/integration/autoland/rev/64bd4d75004d
Fix cyrillic text test after re-adding the HTML preview. r=Honza
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4bab59d6b58f
https://hg.mozilla.org/mozilla-central/rev/edbc03f93f6a
https://hg.mozilla.org/mozilla-central/rev/64bd4d75004d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago11 months ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59

Updated

11 months ago
Keywords: dev-doc-needed

Comment 58

11 months ago
Hi,
This bug is marked as solved but I cannot see any difference. Is this anything to do to have the render ?
Thank you,

Comment 59

11 months ago

This is not solved. I get this with v.57 and had it with 56 as well. 

I even tried with an all new profile to see if it would solve anything. It did not. 

/E
(In reply to Emil from comment #59)
> 
> This is not solved. I get this with v.57 and had it with 56 as well. 
> 
> I even tried with an all new profile to see if it would solve anything. It
> did not. 
> 
> /E

RESOLVED FIXED in Firefox 59 

^^^^^^^

As it says on the top, we have to wait till v59

Comment 61

11 months ago
OK, for the answer. I will wait for a while. I just installed 57. Weight and sea says the whale...

Comment 62

11 months ago
Is possible to uplift this to 58, as suggested in comment #10?
ni regarding the uplift. Developers are really missing this feature.

Sebastian
Flags: needinfo?(brandon.cheng)
Thinking about it, it's probably better to ask Honza about the uplift.

Sebastian
Flags: needinfo?(brandon.cheng) → needinfo?(odvarko)

Updated

11 months ago
Duplicate of this bug: 1150652
(Reporter)

Comment 66

11 months ago
(In reply to Sebastian Zartner [:sebo] from comment #64)
> Thinking about it, it's probably better to ask Honza about the uplift.
I understand, but the patch is rather bigger and I'd recommend to
wait for 59

Honza
Flags: needinfo?(odvarko)

Comment 67

11 months ago
I have reproduced this bug with Nightly 55.0a1 (2017-04-04) on Windows 8.1 , 64 Bit ! 

This bug's fix is Verified with latest Nightly !

Build   ID    20171127100433
User Agent    Mozilla/5.0 (Windows NT 6.3; WOW64; rv:59.0) Gecko/20100101 Firefox/59.0

[bugday-20171122]

Comment 68

11 months ago
Hi, is it fixed? For me, I am using laravel and when I dd (die dump), it still return HTML even though I force it into 500 state
(In reply to yuko.pangestu from comment #68)
> Hi, is it fixed? For me, I am using laravel and when I dd (die dump), it
> still return HTML even though I force it into 500 state

Not sure you're talking about the same thing. This issue asks for a preview for HTML contents within the Response tab when a network request is selected, which got implemented for Firefox 59 (currently on Nightly channel). This is independent of the returned HTTP status code.

Sebastian

Comment 70

11 months ago
(In reply to Sebastian Zartner [:sebo] from comment #69)
> (In reply to yuko.pangestu from comment #68)
> > Hi, is it fixed? For me, I am using laravel and when I dd (die dump), it
> > still return HTML even though I force it into 500 state
> 
> Not sure you're talking about the same thing. This issue asks for a preview
> for HTML contents within the Response tab when a network request is
> selected, which got implemented for Firefox 59 (currently on Nightly
> channel). This is independent of the returned HTTP status code.
> 
> Sebastian

Hi Sebastian, yeah basically I am talking about HTML render, but I don't know that Mozzila has nightly, which I currently download right now, However I have 3 firefox on my computer, regular, quantum and nightly, should I keep them both? Or for development purpose I can uninstall the quantum?

Thanks

Comment 71

11 months ago
Can this be patched in before Firefox 59? This is a pretty bad bug for web devs.
(In reply to directrix1@gmail.com from comment #71)
> Can this be patched in before Firefox 59? This is a pretty bad bug for web
> devs.

I already asked. See the answer in comment 66.

Sebastian

Comment 73

10 months ago
For anyone else asking about patching this in before v59, know that you can get Firefox v59 *now* at https://www.mozilla.org/en-US/firefox/channel/desktop/

Developer mode is Release + 1, while Nightly is Release + 2. As of now we are on Firefox v57 still, so you'll want to grab Nightly. Or if you can wait, v59 will hit Release channel in mid-March of 2018.
I've documented this:

Added a section to the Network Monitor page to show how this feature works:
https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor#HTML_preview

Added a note to the FX59 rel notes
https://developer.mozilla.org/en-US/Firefox/Releases/59#Changes_for_web_developers

Let me know if this is OK. Thanks!

One more question — I also noticed that the Stack trace tab wasn't documented; what version of Fx was that added in?
Flags: needinfo?(odvarko)
Keywords: dev-doc-needed → dev-doc-complete
(Reporter)

Comment 75

9 months ago
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #74)
> I've documented this:
> 
> Added a section to the Network Monitor page to show how this feature works:
> https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor#HTML_preview
> 
> Added a note to the FX59 rel notes
> https://developer.mozilla.org/en-US/Firefox/Releases/
> 59#Changes_for_web_developers
> 
> Let me know if this is OK. Thanks!
Looks great, thanks!

> One more question — I also noticed that the Stack trace tab wasn't
> documented; what version of Fx was that added in?
Bug 1350234, Firefox 55

Honza
Flags: needinfo?(odvarko)

Comment 76

9 months ago
I'd like to say thank you for re-adding this feature, its very helpful. However, i'd like to ask that the requirement for the Content-type header to be html before the preview will be displayed be removed.

The use case being that if i'm developing an API that returns a JSON response, and theres a error in my code, the framework will generate a response in html but the content-type header for json will remain, and thus the html preview will not appear and then i cannot view easily the error generated by the framework.

Thank you.
(In reply to Jan Honza Odvarko [:Honza] from comment #75)
> (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #74)

> 
> > One more question — I also noticed that the Stack trace tab wasn't
> > documented; what version of Fx was that added in?
> Bug 1350234, Firefox 55

Cool, thanks. I've made sure that tab is documented too on the page, with the correct version of Fx cited for when it was added.
(Reporter)

Comment 78

9 months ago
(In reply to Tam Denholm from comment #76)
> I'd like to say thank you for re-adding this feature, its very helpful.
> However, i'd like to ask that the requirement for the Content-type header to
> be html before the preview will be displayed be removed.
> 
> The use case being that if i'm developing an API that returns a JSON
> response, and theres a error in my code, the framework will generate a
> response in html but the content-type header for json will remain, and thus
> the html preview will not appear and then i cannot view easily the error
> generated by the framework.
Please, file a new bug. This one is already closed, thanks!
Honza

Updated

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