Oxide

Merge lp://qastaging/~unity-team/oxide/locationBarHeightOnScreenChange into lp://qastaging/~oxide-developers/oxide/oxide.trunk

  1. locationBarHeightOnScreenChange
  2. Merge into oxide.trunk
Proposed by Gerry Boland
Status: Merged
Merged at revision: 1462
Proposed branch: lp://qastaging/~unity-team/oxide/locationBarHeightOnScreenChange
Merge into: lp://qastaging/~oxide-developers/oxide/oxide.trunk
Diff against target: 80 lines (+17/-2)
3 files modified
qt/core/browser/oxide_qt_contents_view.cc (+6/-0)
qt/core/browser/oxide_qt_web_view.cc (+8/-2)
qt/core/browser/oxide_qt_web_view.h (+3/-0)
To merge this branch: bzr merge lp://qastaging/~unity-team/oxide/locationBarHeightOnScreenChange
Reviewer Review Type Date Requested Status
Chris Coulson Needs Fixing
Michał Sawicz (community) Needs Fixing
Review via email: mp+293278@code.qastaging.launchpad.net

Commit message

On screen scale change, need to recalculate the height of the location bar in terms of the new scale

To post a comment you must log in.
Revision history for this message
Michał Sawicz (saviq) wrote :

../../../../qt/core/browser/oxide_qt_web_view.cc: In member function ‘virtual void oxide::qt::WebView::updateLocationBarHeight() const’:
../../../../qt/core/browser/oxide_qt_web_view.cc:942:44: error: passing ‘const oxide::qt::WebView’ as ‘this’ argument discards qualifiers [-fpermissive]
   setLocationBarHeight(location_bar_height_);
                                            ^
../../../../qt/core/browser/oxide_qt_web_view.cc:934:6: note: in call to ‘virtual void oxide::qt::WebView::setLocationBarHeight(int)’
 void WebView::setLocationBarHeight(int height) {

:/

review: Needs Fixing
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Thanks, I've left a couple of comments inline

review: Needs Fixing
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Thanks, there's a couple of bits that can be deleted now (see the comments inline)

review: Needs Fixing
Revision history for this message
Gerry Boland (gerboland) wrote :

Yep, done that (hadn't pushed), but now the offset of the location bar is wrong. Investigating (and marked the MP WIP, to avoid annoying you until I'm ready)

Revision history for this message
Gerry Boland (gerboland) wrote :

Chris has tested and not seeing the issue I was finding. I blame a deployment problem on my end. I'm going to double-check it, but this should be good to go

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: