Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29740563: Noissue - In the devtools panel, preserve space for missing document domain (Closed)

Created:
April 2, 2018, 6:47 p.m. by Sebastian Noack
Modified:
April 5, 2018, 12:36 a.m.
Reviewers:
Thomas Greiner
Visibility:
Public.

Description

Noissue - In the devtools panel, preserve space for missing document domain

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -4 lines) Patch
M background.js View 1 chunk +1 line, -1 line 0 comments Download
M devtools-panel.html View 1 chunk +1 line, -1 line 4 comments Download
M devtools-panel.js View 2 chunks +3 lines, -2 lines 0 comments Download

Messages

Total messages: 5
Sebastian Noack
Currently, if the document domain is empty the URL is vertically aligned to the middle ...
April 2, 2018, 6:51 p.m. (2018-04-02 18:51:29 UTC) #1
Thomas Greiner
https://codereview.adblockplus.org/29740563/diff/29740564/devtools-panel.html File devtools-panel.html (right): https://codereview.adblockplus.org/29740563/diff/29740564/devtools-panel.html#newcode91 devtools-panel.html:91: <div class="domain">&nbsp;</div> This issue is purely cosmetical so, instead ...
April 3, 2018, 12:39 p.m. (2018-04-03 12:39:43 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29740563/diff/29740564/devtools-panel.html File devtools-panel.html (right): https://codereview.adblockplus.org/29740563/diff/29740564/devtools-panel.html#newcode91 devtools-panel.html:91: <div class="domain">&nbsp;</div> On 2018/04/03 12:39:43, Thomas Greiner wrote: > ...
April 3, 2018, 11:21 p.m. (2018-04-03 23:21:56 UTC) #3
Thomas Greiner
https://codereview.adblockplus.org/29740563/diff/29740564/devtools-panel.html File devtools-panel.html (right): https://codereview.adblockplus.org/29740563/diff/29740564/devtools-panel.html#newcode91 devtools-panel.html:91: <div class="domain">&nbsp;</div> On 2018/04/03 23:21:56, Sebastian Noack wrote: > ...
April 4, 2018, 12:34 p.m. (2018-04-04 12:34:01 UTC) #4
Sebastian Noack
April 5, 2018, 12:36 a.m. (2018-04-05 00:36:10 UTC) #5
This change is no longer relevant. After #6543 and #6544, all requests that end
up being logged should have a document domain.

https://codereview.adblockplus.org/29740563/diff/29740564/devtools-panel.html
File devtools-panel.html (right):

https://codereview.adblockplus.org/29740563/diff/29740564/devtools-panel.html...
devtools-panel.html:91: <div class="domain">&nbsp;</div>
On 2018/04/04 12:34:01, Thomas Greiner wrote:
> In this case I don't see how defining how elements should look like in CSS
> is a hack.

It's a hack to superfluously define a height on the element. This seems much
more error-prone than the approach with the &nbsp;. Currently, the element's
height is defined by the font-size/line-height of its content. If we now
explicitly define a height for the element itself and later change the size of
the content, we have to remember updating the height of the container as well,
otherwise the layout is off.

Powered by Google App Engine
This is Rietveld