871652 - Add clipboard actionbar support in editing mode

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Lucas Rocha (:lucasr)]

Right now the code to show/hide the clipboard toolbar is commented out in the fig branch.

Comment 1

[:Margaret Leibovic]

Do you remember what the issues were around this and why it was commented out?

I just tried un-commenting the mActivity.getActionBar().show/hide calls in BrowserToolbar, and I got an NPE because mActivity.getActionBar() was null.

I also see this method BrowserApp.getActionBarLayout() that's in fig but not in m-c:
http://hg.mozilla.org/projects/fig/annotate/a0890fdc1346/mobile/android/base/BrowserApp.java#l927

So basically, I don't actually know how to fix this bug and I was wondering if you do :)

Comment 2

[Lucas Rocha (:lucasr)]

To be very honest, I don't remember the exact context of this feature. I think sriram implemented this in the original code.

Comment 3

[Sriram Ramasubramanian [:sriram]]

(In reply to :Margaret Leibovic from comment #1)
> Do you remember what the issues were around this and why it was commented
> out?
> 
> I just tried un-commenting the mActivity.getActionBar().show/hide calls in
> BrowserToolbar, and I got an NPE because mActivity.getActionBar() was null.
> 
> I also see this method BrowserApp.getActionBarLayout() that's in fig but not
> in m-c:
> http://hg.mozilla.org/projects/fig/annotate/a0890fdc1346/mobile/android/base/
> BrowserApp.java#l927
> 
> So basically, I don't actually know how to fix this bug and I was wondering
> if you do :)

There is no ActionBar in BrowserApp. So, if you call "getActionBar()" it's going to be null and would crash.

I fixed the bug 889562 by removing all those bits of code that we force to show an ActionBar. Android does it for free. The only issue from the BrowserApp's side is that, BrowserToolbar also has a long press menu (that's such a biggg biggg UI problem! who'll know that one can long press on url-bar?? that too when android is moving away from long press :( :( :enough-of-sriram's-rant: ). So, there is a race between both and BrowserToolbar wins. If we have a different approach there -- if in editing mode, don't show our long press menu, if not, show it -- this would work fine, by itself.

Comment 4

[Lucas Rocha (:lucasr)]

Attachment: Disable toolbar's context menu while in editing mode

Comment 5

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 789620 [details] [diff] [review]
Disable toolbar's context menu while in editing mode

Review of attachment 789620 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks a bunch for restoring this.

Comment 6

[Lucas Rocha (:lucasr)]

Pushed: http://hg.mozilla.org/projects/fig/rev/7fb187dbff36

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7fb187dbff36