SSIA
Created attachment 146005 [details] Patch
Created attachment 146006 [details] Patch
Comment on attachment 146006 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146006&action=review LGTM (two comments below) :) - Maybe Tor Arne can have a look too and r+? > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:25 > + anchors.margins: 10 As discussed with Tor Arne on irc, anchors { ... } would be the preferred way. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:37 > + Same as above regarding the anchors.
Created attachment 146007 [details] Patch
Created attachment 146010 [details] Patch
Comment on attachment 146010 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146010&action=review Looks good in general. but there are a couple of problems with the property access, which leads to accessing undefined properties and hence warnings. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:78 > + font.family: parent.fontFamily This does not seem to work properly as the font.family is defined in the parent's grandparent (i.e. the Text's parent is the inner column, the inner Column's parent is the outer Column, the outer columns parent is the Item which has the property) - MiniBrowser put's out some warnings about undefined properties on the console. You could either go up the hierarchy (parent.parent.parent) or assign an id to the Item (e.g. "infoColumContainer"?) to access the properties. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:79 > + color: parent.fontColor The above applies to fontColor as well. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:86 > + font.family: parent.fontFamily Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:87 > + color: parent.fontColor Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:91 > + font.family: parent.fontFamily Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:92 > + color: parent.fontColor Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:96 > + font.family: parent.fontFamily Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:97 > + color: parent.fontColor Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:104 > + font.family: parent.fontFamily Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:105 > + color: parent.fontColor Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:109 > + font.family: parent.fontFamily Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:110 > + color: parent.fontColor Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:114 > + font.family: parent.fontFamily Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:115 > + color: parent.fontColor Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:122 > + font.family: parent.fontFamily Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:123 > + color: parent.fontColor Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:127 > + font.family: parent.fontFamily Same as above. > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:128 > + color: parent.fontColor Same as above.
Created attachment 146018 [details] Patch
Looks good to me.
Landed in r119710