Closed
Bug 431843
Opened 17 years ago
Closed 17 years ago
Panning - smooth movement
Categories
(Firefox for Android Graveyard :: Panning/Zooming, enhancement, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
fennec1.0m4
People
(Reporter: christian.bugzilla, Assigned: Gavin)
References
Details
Attachments
(1 file, 1 obsolete file)
|
51.49 KB,
patch
|
Details | Diff | Splinter Review |
|
Reporter | |
Updated•17 years ago
|
Assignee: nobody → gavin.sharp
Priority: -- → P1
Target Milestone: --- → Fennec M4
|
||
Updated•17 years ago
|
Flags: blocking-fennec1.0+
|
Assignee | |
Comment 1•17 years ago
|
||
This patch has two main parts:
1) removal of the deckpage/tab support (r?mfinkle)
2) implementation of canvas-based pan/zoom based on stuart's PoC (r?stuart)
Stuart's original proof of concept used a hardcoded width/height canvas/browser, but I've switched to using a flex-based approach, to support different windows sizes, and resizes. This required an ugly hack to let the canvas know what it's real size is, and using a deck that will flex instead of just two stacks, but it doesn't seem to negatively affect performance, so I figured we can revisit later. margins are used to move the canvas instead of left/right to avoid triggering absolute positioning layout, which seems to be slower.
Attachment #326007 -
Flags: review?(pavlov)
Attachment #326007 -
Flags: review?(mark.finkle)
|
|
Assignee | |
Comment 2•17 years ago
|
||
Comment on attachment 326007 [details] [diff] [review]
patch
>diff -r 2231628c6a5d app/mobile.js
>+pref("browser.ui.cursor", true);
Ignore this change, of course...
|
||
Comment 3•17 years ago
|
||
Comment on attachment 326007 [details] [diff] [review]
patch
>diff -r 2231628c6a5d app/mobile.js
>-pref("browser.ui.cursor", false);
>+pref("browser.ui.cursor", true);
Do you want to enable the cursor on the device? or is this just from your PC setup?
>diff -r 2231628c6a5d chrome/content/browser.js
>-function ProgressController(aBrowser) {
>+
>+ // FIXME: until we can get proper canvas repainting hooked up, update the canvas every 300ms
>+ //var tabbrowser = this._tabbrowser;
>+ //setTimeout(function () {
>+ // tabbrowser.updateCanvasState();
>+ //}, 300);
Yeah, this is short term, but deckbrowser might have a better place for it. No need to let this "detail" leak from the deckbrowser impl.
> },
>
> onStateChange : function(aWebProgress, aRequest, aStateFlags, aStatus) {
>@@ -355,13 +340,14 @@
> LocationBar.update(TOOLBARSTATE_LOADING);
> if (HUDBar)
> HUDBar.update(TOOLBARSTATE_LOADING);
>+ this._tabbrowser.updateCanvasState();
Ok, now I see the issue - we have to leak other things too. We should eventually move this progresscontroller into deckbrowser.
>diff -r 2231628c6a5d chrome/content/deckbrowser.xml
Wow, the way that diff was intermingled was crazy - but it looked ok to me
There are some <command>s you could remove too, right?
Attachment #326007 -
Flags: review?(mark.finkle) → review+
|
|
Assignee | |
Comment 4•17 years ago
|
||
(In reply to comment #3)
> Do you want to enable the cursor on the device? or is this just from your PC
> setup?
Yeah, this is just a local change that snuck in.
> Ok, now I see the issue - we have to leak other things too. We should
> eventually move this progresscontroller into deckbrowser.
Could do - not sure it's worth duplicating progresscontrollers just to separate deckbrowser and the rest of the fennec code, though.
> There are some <command>s you could remove too, right?
Could be, will look into that.
Also need to investigate a problem romaxa pointed out with panning on hs.fi (constraint code definitely needs some tweaking).
|
||
Comment 5•17 years ago
|
||
Comment on attachment 326007 [details] [diff] [review]
patch
this looks mostly right -- i'll do a more full review of this code when i get back from vacation
Attachment #326007 -
Flags: review?(pavlov) → review+
|
|
Assignee | |
Comment 6•17 years ago
|
||
- Fixes hs.fi, by using the maximum scrollable width of <html> or <body> (same as the existing ZoomController code)
- Removes unneeded <command>s
Attachment #326007 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 7•17 years ago
|
||
Pushed to mobile-browser:
http://hg.mozilla.org/mobile-browser/index.cgi/rev/d31f553a503a
Going to call this fixed, still work to be done in followups though.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
|
||
Comment 8•16 years ago
|
||
this has been in for a while and verified with beta3
|
|
||
Updated•16 years ago
|
Status: RESOLVED → VERIFIED
|
||
Updated•15 years ago
|
Component: General → Panning/Zooming
You need to log in
before you can comment on or make changes to this bug.
Description
•