Closed Bug 905563 Opened 6 years ago Closed 6 years ago

[Buri][Browser]The browser should supply "open link in new tab" button when long tap a new

Categories

(Firefox OS Graveyard :: Gaia::Browser, defect, P2)

defect

Tracking

(blocking-b2g:koi+)

RESOLVED FIXED
blocking-b2g koi+

People

(Reporter: sync-1, Unassigned)

Details

(Whiteboard: [sprintready])

Attachments

(2 files)

Mozilla build ID:20130806071254
 
 Created an attachment (id=473164)
 logcat
 
 DEFECT DESCRIPTION:
 
  The browser should  supply  "open link in new tab" button when long tap a new
 
  REPRODUCING PROCEDURES:
 
  1.Open Browser app
 
  2.Open a web page : news.google.com
 
  3.long tap a news-->MS can't to bring up the "open link in new tab" menu-->KO
 
  EXPECTED BEHAVIOUR:
  
  The MS should to bing up the "open link in new tab" menu
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
  mid
  REPRODUCING RATE:
  5/5
  For FT PR, Please list reference mobile's behavior:
 
  Firefox browser in Android behavior:browser will to bing up the "open link in new tab" menu when long tap a news
Clone from brother
Attached file logcat
This only seems to happen on this page and may be caused by JavaScript on the page, but marking as sprintready to look into it.
Whiteboard: [sprintready]
What does sprintready mean?  Ready to fix on next release?
I've been trying on a couple pages and can't seem to action with a long press (on m.bbc.co.uk/news for instance in trying to open an article in a new tab)
A Pivotal Tracker story has been created for this Bug: http://www.pivotaltracker.com/story/show/55557746
I've been trying on a couple pages and can't seem to action with a long press (on m.bbc.co.uk/news for instance in trying to open an article in a new tab)
news.google.com for instance.
Use firebug extension, I find the HTML of news.google.com  not using <a> ....</a>
as a link. So, our browser doesn't recognise the link.(can't open in a new tab)

The same problem while we save a picture on the web. When long press a picture shown on the 
web, which HTML isn't IMG taged...

I think we should add more to know they are actually a picture. Just like Fx for Android does.

BTW, I see same problem on master branch.
Attached file PR to master
Hi Dale,

I notice that if I long press a <a><span>link</span></a>, the nodeName would be |a| instead of |A|. I'm not quite sure if it's expected or not.

And one more thing, it's also fix a small regression of bug 907056, I just realize d whenever I long-press the content of browser, it will trigger mozbrowsercontextmenu event, so I add a line to set contextMenuHasCalled to false if no menuitem is created.
Attachment #799308 - Flags: review?(dale)
Dears, is it possible to implement this feature in v1.1?
blocking-b2g: --- → leo?
This feature is available in 1.1, this is fixing a relative edge case, I dont think we have control over leo flags at this point but I would expect this patch almost certainly wont be uplifted to 1.1 which has been long closed afaik
Comment on attachment 799308 [details]
PR to master

Hey George, cheers for this, good catch

So nodeName in html is supposed to be uppercase, either way 'A' is expected so instead of testing for 'a', we should just uppercase the nodeName prior to the check so 'a' and 'A' both work (https://developer.mozilla.org/en-US/docs/Web/API/element.tagName?redirectlocale=en-US&redirectslug=DOM%2Felement.tagName)

Also especially as we managed to introduce a regression, we are starting to require integration tests for features and bug fixes where it makes sense (particularly regressions), so clearing the review on this and if you could add tests that would be great. 

For this test we will want a new contextmenu_test.js, you can use https://github.com/mozilla-b2g/gaia/blob/master/apps/browser/test/marionette/navigate_test.js as an example of how to load a new page, we can start with a very simple load a page, fire a context menu, ensure context menu fires callback to start with, if you hit any problems with the integration tests just ping on here / irc

Cheers
Dale
Attachment #799308 - Flags: review?(dale)
QA,

Please check if this is reproducible on 1.0.1.
Keywords: qawanted
Comment on attachment 799308 [details]
PR to master

Hi Dale,

Test updated for 'open new page' case. Please kindly check.
Attachment #799308 - Flags: review?(dale)
QA Contact: sparsons
In response to comment 14

The issue does reproduce on v1.0.1,long pressing any article on news.google.com does not give the user option to open the article in new tab,whereas long pressing on any article on m.bbc.co.uk gives user the option to open the link in new tab.

Environmental Variables
Build ID: 20130906043205
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/9c62297d11b0
Gaia: 054cdc27404e2daca91d3065d9783681032b2151
Platform Version: 18.0
Keywords: qawanted
Comment on attachment 799308 [details]
PR to master

Great thanks
Attachment #799308 - Flags: review?(dale) → review+
https://github.com/mozilla-b2g/gaia/commit/083c8410a44b52699b3cff4065f653bd38033e4c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
New feature - will not take in 1.1
blocking-b2g: leo? → koi?
blocking-b2g: koi? → koi+
You need to log in before you can comment on or make changes to this bug.